mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 22:16:23 +00:00
Clojure LSP: improve module resolution by parsing source path providers
This commit is contained in:
1 parent
42b5500efc
commit
077dba4d21
8 files changed
+291
-7
No files matched your search
@@ -29,6 +29,7 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
- Add **HTML** (experimental) using `vscode-html-language-server` from the `vscode-langservers-extracted` npm package. Provides in-file element/id symbols via documentSymbol; cross-file references are not meaningful for HTML. Also used as a companion server by the Angular LS for plain HTML documentSymbol support.
|
||||
- Add **SCSS / Sass / CSS** (experimental) using [some-sass-language-server](https://github.com/wkillerud/some-sass). Handles `.scss`, `.sass`, and `.css` through one server, with full `@use`/`@forward` workspace-wide go-to-definition and find-references for variables, mixins, and functions across Sass files. The `.css` path uses the same `vscode-css-languageservice` engine that powers the standalone CSS LS; CSS feature toggles default off upstream and are flipped on at startup so symbols, hover, completion, and syntax-level diagnostics work for plain CSS as well.
|
||||
- Add support for more filenames to be considered by ccls and clangd.
|
||||
- Clojure (`clojure-lsp`): Fix incomplete `find_referencing_symbols` results in multi-module monorepos. clojure-lsp only discovers source paths from the descriptor at the workspace root and does not recurse for sub-module `deps.edn` / `project.clj` / `shadow-cljs.edn` / `bb.edn` files, so references in sibling modules were silently missed until those files happened to be opened by `find_symbol` / `get_symbols_overview`. Serena now scans the repo for project descriptors at startup and passes the union of their declared source paths to clojure-lsp via `initializationOptions`. Project-local `.lsp/config.edn` files are honoured as-is (no override). New `ls_specific_settings.clojure` keys: `source_paths` (explicit override) and `config_edn_path` (parse `:source-paths` from a user-supplied config file).
|
||||
|
||||
* Hooks:
|
||||
- `serena-hooks auto-approve` now also emits an `allow` decision when Claude Code reports
|
||||
|
||||
@@ -318,6 +318,29 @@ Supported settings:
|
||||
|---|---|---|
|
||||
| `ls_path` | managed download | Override the `clojure-lsp` executable path. |
|
||||
| `clojure_lsp_version` | `2026.02.20-16.08.58` | Override the `clojure-lsp` release version Serena downloads when `ls_path` is not set. |
|
||||
| `source_paths` | scanned from project descriptors (or unset if a project-local `.lsp/config.edn` is found) | Explicit list of repo-root-relative source paths to inject into clojure-lsp's `initializationOptions`. Use this when the auto-discovery picks up too few or too many paths. |
|
||||
| `config_edn_path` | unset | Path to a `config.edn` file whose `:source-paths` entry should be parsed and injected. Useful when the project's clojure-lsp config lives outside the standard `.lsp/config.edn` location. |
|
||||
|
||||
**Why this exists**: clojure-lsp discovers source paths only from the project descriptor at the workspace root (root `deps.edn` / `project.clj` / `shadow-cljs.edn` / `bb.edn`) and does not recurse for sub-module descriptors. In multi-module monorepos (e.g. `common/` + `frontend/` + `backend/` layouts), this means references in sibling modules are silently missed by `find_referencing_symbols` until a tool call happens to open one of their files. Serena works around this by walking the repo for project descriptors at startup and passing the union of their declared source paths to clojure-lsp via `initializationOptions["source-paths"]`.
|
||||
|
||||
**Resolution order** (first match wins):
|
||||
|
||||
1. `source_paths` setting — explicit override.
|
||||
2. `config_edn_path` setting — Serena parses `:source-paths` from the supplied file.
|
||||
3. `<repo>/.lsp/config.edn` exists — Serena injects nothing; clojure-lsp reads the file natively, so hand-tuned project configs are never clobbered.
|
||||
4. Walk the repo for project descriptors and synthesise a source-paths list from their declared `:paths` / `:extra-paths` / `:source-paths` (skipping `.git`, `.clj-kondo`, `.lsp`, `.cpcache`, `node_modules`, `target`, `out`, `dist`).
|
||||
|
||||
Example — a monorepo without a `.lsp/config.edn`, where you want to override what Serena scanned:
|
||||
|
||||
```yaml
|
||||
ls_specific_settings:
|
||||
clojure:
|
||||
source_paths:
|
||||
- "common/src"
|
||||
- "common/test"
|
||||
- "frontend/src"
|
||||
- "backend/src"
|
||||
```
|
||||
|
||||
#### C/C++ (`clangd`)
|
||||
|
||||
|
||||
+1
-1
@@ -359,4 +359,4 @@ markers = [
|
||||
skip = '.git*,*.svg,*.lock,*.min.*'
|
||||
check-hidden = true
|
||||
ignore-regex = '\.\w+'
|
||||
ignore-words-list = 'paket'
|
||||
ignore-words-list = 'paket,EDN'
|
||||
@@ -5,6 +5,7 @@ Provides Clojure specific instantiation of the LanguageServer class. Contains va
|
||||
import logging
|
||||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import threading
|
||||
@@ -84,14 +85,95 @@ class ClojureLSP(SolidLanguageServer):
|
||||
You can pass the following entries in ``ls_specific_settings["clojure"]``:
|
||||
- clojure_lsp_version: Override the pinned clojure-lsp version downloaded
|
||||
by Serena (default: the bundled Serena version).
|
||||
- source_paths: Explicit list of source paths (repo-root-relative) to
|
||||
inject into clojure-lsp's ``initializationOptions``. Skips both the
|
||||
``.lsp/config.edn`` lookup and the project-tree scan.
|
||||
- config_edn_path: Path to a ``config.edn`` file whose ``:source-paths``
|
||||
entry should be parsed and injected. Skips the project-tree scan but
|
||||
is itself skipped if ``source_paths`` is also set.
|
||||
|
||||
Source-path resolution order (first match wins):
|
||||
1. ``source_paths`` setting (explicit override)
|
||||
2. ``config_edn_path`` setting (explicit config file)
|
||||
3. ``<repo>/.lsp/config.edn`` exists → trust it (clojure-lsp reads it
|
||||
natively, so we inject nothing)
|
||||
4. Walk the repo for ``deps.edn`` / ``project.clj`` / ``shadow-cljs.edn``
|
||||
/ ``bb.edn`` and synthesise a source-paths list from their declared
|
||||
``:paths`` / ``:extra-paths`` / ``:source-paths``.
|
||||
"""
|
||||
|
||||
CLOJURE_LSP_ALLOWED_HOSTS = CLOJURE_LSP_ALLOWED_HOSTS
|
||||
|
||||
# Files that mark a directory as the root of a Clojure (sub-)project. Used both to detect
|
||||
# multi-module monorepos when synthesising clojure-lsp's `source-paths` and to walk the
|
||||
# tree extracting declared paths.
|
||||
_PROJECT_DESCRIPTOR_FILENAMES = ("deps.edn", "project.clj", "shadow-cljs.edn", "bb.edn")
|
||||
|
||||
# Best-effort EDN extraction (full EDN parsing would be overkill for discovery hints):
|
||||
# matches `:paths […]`, `:extra-paths […]` and `:source-paths […]` followed by a vector of strings.
|
||||
_PATHS_VECTOR_RE = re.compile(r":(?:extra-paths|source-paths|paths)\s*\[([^\]]*)\]")
|
||||
_QUOTED_STRING_RE = re.compile(r'"([^"]+)"')
|
||||
|
||||
# Clojure-specific directories worth pruning
|
||||
_IGNORED_DIRS = frozenset({".clj-kondo", ".lsp", ".cpcache", "node_modules", "target", "out", "dist"})
|
||||
|
||||
@override
|
||||
def is_ignored_dirname(self, dirname: str) -> bool:
|
||||
ignored_dirs = [".clj-kondo", ".lsp", ".cpcache"]
|
||||
return super().is_ignored_dirname(dirname) or dirname in ignored_dirs
|
||||
return super().is_ignored_dirname(dirname) or dirname in self._IGNORED_DIRS
|
||||
|
||||
@staticmethod
|
||||
def _parse_project_descriptor_paths(descriptor_path: str) -> list[str]:
|
||||
"""Extracts source-path string entries from a Clojure project descriptor (best-effort).
|
||||
|
||||
:param descriptor_path: absolute path to a `deps.edn`, `project.clj`, `shadow-cljs.edn` or `bb.edn`
|
||||
:return: the path strings declared under `:paths`, `:extra-paths` or `:source-paths` keys,
|
||||
in the order they appear; relative to the descriptor's own directory
|
||||
"""
|
||||
try:
|
||||
text = pathlib.Path(descriptor_path).read_text(encoding="utf-8")
|
||||
except OSError as e:
|
||||
log.debug(f"Could not read project descriptor {descriptor_path}: {e}")
|
||||
return []
|
||||
|
||||
declared: list[str] = []
|
||||
for vector_match in ClojureLSP._PATHS_VECTOR_RE.finditer(text):
|
||||
declared.extend(ClojureLSP._QUOTED_STRING_RE.findall(vector_match.group(1)))
|
||||
return declared
|
||||
|
||||
def _collect_source_paths(self) -> list[str]:
|
||||
"""Walks the repo to discover all declared Clojure source paths across modules.
|
||||
|
||||
Workaround for a clojure-lsp limitation: it discovers source paths only from the
|
||||
descriptor at the workspace root and does not recurse for sub-module descriptors,
|
||||
even when additional ``workspaceFolders`` are advertised. For multi-module monorepos
|
||||
this means references in sibling modules are silently missed until the user happens
|
||||
to open one of their files. We pass the union of declared paths to clojure-lsp via
|
||||
``initializationOptions["source-paths"]`` — equivalent to what users of such monorepos
|
||||
otherwise have to write manually in ``.lsp/config.edn``.
|
||||
|
||||
:return: deduplicated, repo-root-relative, forward-slash-normalised source paths from
|
||||
every project descriptor found in the tree (pruning via :py:meth:`is_ignored_dirname`);
|
||||
falls back to ``["src"]`` if nothing is discovered, matching the clojure-lsp default
|
||||
"""
|
||||
repository_absolute_path = self.repository_root_path
|
||||
discovered: list[str] = []
|
||||
for dirpath, dirnames, filenames in os.walk(repository_absolute_path):
|
||||
# prune ignored dirs at the parent level so we never descend into node_modules,
|
||||
# target, .git etc. — much cheaper than scandir-ing them and filtering after
|
||||
dirnames[:] = [d for d in dirnames if not self.is_ignored_dirname(d)]
|
||||
|
||||
# for each descriptor at this level, resolve declared paths back to the repo root
|
||||
module_rel = os.path.relpath(dirpath, repository_absolute_path)
|
||||
for descriptor in self._PROJECT_DESCRIPTOR_FILENAMES:
|
||||
if descriptor in filenames:
|
||||
for declared_path in self._parse_project_descriptor_paths(os.path.join(dirpath, descriptor)):
|
||||
if module_rel == ".":
|
||||
resolved = declared_path
|
||||
else:
|
||||
resolved = os.path.normpath(os.path.join(module_rel, declared_path))
|
||||
discovered.append(resolved.replace(os.sep, "/"))
|
||||
|
||||
return list(set(discovered)) or ["src"]
|
||||
|
||||
@classmethod
|
||||
def _runtime_dependencies(cls, version: str) -> RuntimeDependencyCollection:
|
||||
@@ -193,10 +275,49 @@ class ClojureLSP(SolidLanguageServer):
|
||||
def _create_launch_command(self, core_path: str) -> list[str]:
|
||||
return [core_path]
|
||||
|
||||
@staticmethod
|
||||
def _get_initialize_params(repository_absolute_path: str) -> InitializeParams:
|
||||
def _resolve_source_paths(self) -> list[str] | None:
|
||||
"""Determines whether to inject ``source-paths`` into clojure-lsp's init options.
|
||||
|
||||
:return: the list of repo-root-relative source paths to inject, or ``None`` to inject
|
||||
nothing (because clojure-lsp will read the project's own ``.lsp/config.edn`` natively).
|
||||
See the class docstring for the precedence order.
|
||||
"""
|
||||
# explicit user override of source paths wins outright
|
||||
explicit_paths = self._custom_settings.get("source_paths")
|
||||
if explicit_paths:
|
||||
log.info(f"clojure-lsp source-paths from user setting 'source_paths': {explicit_paths}")
|
||||
return list(explicit_paths)
|
||||
|
||||
# user-supplied config.edn path: parse and extract source paths from it
|
||||
explicit_config_edn = self._custom_settings.get("config_edn_path")
|
||||
if explicit_config_edn:
|
||||
parsed = self._parse_project_descriptor_paths(explicit_config_edn)
|
||||
log.info(f"clojure-lsp source-paths from user setting 'config_edn_path' ({explicit_config_edn}): {parsed}")
|
||||
return parsed or None
|
||||
|
||||
# repo-local .lsp/config.edn: clojure-lsp will read it natively, so we leave it alone
|
||||
# to avoid clobbering hand-tuned configs in projects like penpot
|
||||
repo_config_edn = pathlib.Path(self.repository_root_path) / ".lsp" / "config.edn"
|
||||
if repo_config_edn.is_file():
|
||||
log.info(f"clojure-lsp will read project's own {repo_config_edn} (no source-paths injection)")
|
||||
return None
|
||||
|
||||
# fall back to scanning for project descriptors; this is the workaround for clojure-lsp's
|
||||
# lack of recursion into sub-module deps.edn files in multi-module monorepos
|
||||
scanned = self._collect_source_paths()
|
||||
log.info(f"clojure-lsp source-paths scanned from project descriptors: {scanned}")
|
||||
return scanned
|
||||
|
||||
def _get_initialize_params(self) -> InitializeParams:
|
||||
"""Returns the init params for clojure-lsp."""
|
||||
repository_absolute_path = self.repository_root_path
|
||||
root_uri = pathlib.Path(repository_absolute_path).as_uri()
|
||||
source_paths = self._resolve_source_paths()
|
||||
|
||||
initialization_options: dict = {"dependency-scheme": "jar", "text-document-sync-kind": "incremental"}
|
||||
if source_paths is not None:
|
||||
initialization_options["source-paths"] = source_paths
|
||||
|
||||
result = { # type: ignore
|
||||
"processId": os.getpid(),
|
||||
"rootPath": repository_absolute_path,
|
||||
@@ -221,7 +342,7 @@ class ClojureLSP(SolidLanguageServer):
|
||||
},
|
||||
"general": {"positionEncodings": ["utf-16"]},
|
||||
},
|
||||
"initializationOptions": {"dependency-scheme": "jar", "text-document-sync-kind": "incremental"},
|
||||
"initializationOptions": initialization_options,
|
||||
"trace": "off",
|
||||
"workspaceFolders": [{"uri": root_uri, "name": os.path.basename(repository_absolute_path)}],
|
||||
}
|
||||
@@ -268,7 +389,7 @@ class ClojureLSP(SolidLanguageServer):
|
||||
log.info("Starting clojure-lsp server process")
|
||||
self.server.start()
|
||||
|
||||
initialize_params = self._get_initialize_params(self.repository_root_path)
|
||||
initialize_params = self._get_initialize_params()
|
||||
|
||||
log.info("Sending initialize request from LSP client to LSP server and awaiting response")
|
||||
init_response = self.server.send.initialize(initialize_params)
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
(ns test-app.extra
|
||||
(:require [test-app.core :as core]))
|
||||
|
||||
(defn double-product
|
||||
"Computes 2 * a * b by calling core/multiply twice."
|
||||
[a b]
|
||||
(core/multiply 2 (core/multiply a b)))
|
||||
|
||||
(defn triple-product
|
||||
"Computes a * b * c by calling core/multiply twice."
|
||||
[a b c]
|
||||
(core/multiply a (core/multiply b c)))
|
||||
@@ -0,0 +1,2 @@
|
||||
{:paths ["src"]
|
||||
:deps {test-app/test-app {:local/root "../"}}}
|
||||
@@ -0,0 +1,15 @@
|
||||
(ns sub-module-app.consumer
|
||||
"A consumer of test-app.core that lives in a sibling module with its own deps.edn,
|
||||
mimicking the multi-module layout of real-world Clojure projects (e.g. penpot's
|
||||
common/, frontend/, backend/ split). The bug being reproduced: references to
|
||||
symbols defined in test-app.core are missed for files in sibling modules until
|
||||
those files are explicitly opened/indexed via find_symbol or get_symbols_overview."
|
||||
(:require [test-app.core :as core]))
|
||||
|
||||
(defn quadruple-product
|
||||
[a b]
|
||||
(core/multiply 4 (core/multiply a b)))
|
||||
|
||||
(defn quintuple-product
|
||||
[a b]
|
||||
(core/multiply 5 (core/multiply a b)))
|
||||
@@ -0,0 +1,110 @@
|
||||
"""Regression tests for clojure-lsp project-wide indexing.
|
||||
|
||||
Bug: ``find_referencing_symbols`` (and the underlying ``request_references``)
|
||||
returns incomplete cross-file results unless the referencing files have already
|
||||
been opened/indexed during the current session via ``find_symbol`` /
|
||||
``get_symbols_overview``. The result is deterministically partial — it grows as
|
||||
more files happen to get indexed by side-effect — which silently biases call
|
||||
graphs towards files the user has already explored.
|
||||
|
||||
The tests here use a fresh module-scoped language server and probe a symbol
|
||||
whose references live in a file (``extra.clj``) that no other test in this
|
||||
module touches. The references must be returned even though no prior tool call
|
||||
has caused the file to be opened in the LSP.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from serena.util.text_utils import find_text_coordinates
|
||||
from solidlsp.ls import SolidLanguageServer
|
||||
from solidlsp.ls_config import Language
|
||||
from test.conftest import language_tests_enabled
|
||||
from test.solidlsp.conftest import read_repo_file
|
||||
|
||||
from . import CORE_PATH, TEST_APP_PATH
|
||||
|
||||
EXTRA_PATH = str(TEST_APP_PATH / "extra.clj")
|
||||
SUBMODULE_CONSUMER_PATH = str(Path("sub_module") / "src" / "sub_module_app" / "consumer.clj")
|
||||
|
||||
|
||||
@pytest.mark.skipif(not language_tests_enabled(Language.CLOJURE), reason="Clojure tests are disabled")
|
||||
@pytest.mark.clojure
|
||||
class TestClojureProjectIndexing:
|
||||
"""Covers the "indexing leaks through results" bug for clojure-lsp.
|
||||
|
||||
The fixture is module-scoped, so all tests within this class share a single
|
||||
server. We deliberately avoid any operation that would open ``extra.clj``
|
||||
(``get_symbols_overview``, ``find_symbol``, ``open_file``) before asserting
|
||||
on references — otherwise the bug would be masked by the side-effectful
|
||||
indexing of those calls.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.CLOJURE], indirect=True)
|
||||
def test_request_references_includes_unopened_file(self, language_server: SolidLanguageServer) -> None:
|
||||
# locate the definition of `multiply` in core.clj without hardcoding coords
|
||||
core_content = read_repo_file(language_server, CORE_PATH)
|
||||
coords = find_text_coordinates(core_content, r"\(defn (multiply)\b", require_unique=True)
|
||||
assert coords is not None, "Could not locate the 'multiply' definition in core.clj"
|
||||
|
||||
# request references straight away — nothing has caused extra.clj to be opened in the LSP
|
||||
refs = language_server.request_references(CORE_PATH, coords.line, coords.col)
|
||||
ref_paths = {r.get("relativePath", "") for r in refs}
|
||||
|
||||
# extra.clj contains two real call sites (in double-product and triple-product);
|
||||
# they must be returned regardless of whether the file was opened beforehand
|
||||
extra_refs = [r for r in refs if r.get("relativePath", "").endswith("extra.clj")]
|
||||
assert extra_refs, (
|
||||
"Expected references to 'multiply' to include call sites from extra.clj, "
|
||||
f"but got files: {sorted(ref_paths)}. "
|
||||
"This indicates clojure-lsp is only indexing files that have been opened "
|
||||
"during the session, so cross-file references are silently incomplete."
|
||||
)
|
||||
assert len(extra_refs) >= 2, (
|
||||
f"Expected at least 2 references in extra.clj (double-product and triple-product), got {len(extra_refs)}: {extra_refs}"
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.CLOJURE], indirect=True)
|
||||
def test_request_references_includes_unopened_sibling_module_file(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Mirrors the real-world penpot bug repro: ``multiply`` is defined in one
|
||||
module (root ``src/``) and consumed from a sibling module that has its
|
||||
own ``deps.edn`` (``sub_module/``). With clojure-lsp running at the
|
||||
repository root, references to ``multiply`` must include the sibling
|
||||
module's call sites — even when no prior tool call has opened those
|
||||
files in the LSP.
|
||||
"""
|
||||
core_content = read_repo_file(language_server, CORE_PATH)
|
||||
coords = find_text_coordinates(core_content, r"\(defn (multiply)\b", require_unique=True)
|
||||
assert coords is not None, "Could not locate the 'multiply' definition in core.clj"
|
||||
|
||||
refs = language_server.request_references(CORE_PATH, coords.line, coords.col)
|
||||
ref_paths = {r.get("relativePath", "") for r in refs}
|
||||
|
||||
consumer_refs = [
|
||||
r for r in refs if r.get("relativePath", "").replace("\\", "/").endswith("sub_module/src/sub_module_app/consumer.clj")
|
||||
]
|
||||
assert consumer_refs, (
|
||||
"Expected references to 'multiply' to include call sites from the sibling module "
|
||||
"(sub_module/src/sub_module_app/consumer.clj), but got files: "
|
||||
f"{sorted(ref_paths)}. This mirrors the penpot bug: clojure-lsp does not appear to "
|
||||
"index files in sibling modules until they are explicitly opened via find_symbol / "
|
||||
"get_symbols_overview, so reference search returns silently incomplete results."
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.CLOJURE], indirect=True)
|
||||
def test_request_referencing_symbols_includes_unopened_file(self, language_server: SolidLanguageServer) -> None:
|
||||
core_content = read_repo_file(language_server, CORE_PATH)
|
||||
coords = find_text_coordinates(core_content, r"\(defn (multiply)\b", require_unique=True)
|
||||
assert coords is not None, "Could not locate the 'multiply' definition in core.clj"
|
||||
|
||||
# request_referencing_symbols is what the user-facing find_referencing_symbols tool calls into
|
||||
refs = language_server.request_referencing_symbols(CORE_PATH, coords.line, coords.col)
|
||||
|
||||
referencing_names = {ref.symbol.get("name", "") for ref in refs if hasattr(ref, "symbol")}
|
||||
# double-product and triple-product live in extra.clj and both call multiply
|
||||
assert "double-product" in referencing_names or "triple-product" in referencing_names, (
|
||||
"Expected referencing symbols to include functions from extra.clj (double-product / triple-product), "
|
||||
f"but got: {sorted(referencing_names)}. "
|
||||
"extra.clj appears not to have been indexed by clojure-lsp because no prior tool call opened it."
|
||||
)
|
||||
Reference in new issue
Block a user