mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-05 06:14:06 +00:00
fix(erlang): address functions by name/arity without the name path separator
Erlang LS identifies functions, types and parameterised macros as `name/arity`, but `/` separates the components of a Serena name path, so `create_user/4` was parsed as "symbol `4` nested inside `create_user`" and could never match -- not even via the name path Serena itself reported for the symbol. Browsing still worked, but find_referencing_symbols, replace_symbol_body and insert_after_symbol were unusable on Erlang functions. Normalize the name to `create_user#4` instead. `#` cannot occur in an unquoted Erlang atom, so it can never collide with a real name, unlike `@`. The arity is kept rather than stripped because it is part of a function's identity in Erlang: create_order/3 and create_order/2 are different functions and may coexist in one module. Also document the "no `/` in symbol names" rule on _normalize_symbol_name, which is what the next backend for such a language needs to know.
This commit is contained in:
1 parent
9fc8033ece
commit
29d07d4f6b
5 files changed
+148
-2
No files matched your search
@@ -38,6 +38,11 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
- Fix: F#'s `module <Name>` declarations reported a `selectionRange` pointing at the `module`
|
||||
keyword instead of at `<Name>`, so looking up hover/references from a module symbol's position
|
||||
returned the keyword's own docs instead of the module's #925
|
||||
- Fix: Erlang functions could not be addressed by any tool taking an exact name path, because
|
||||
Erlang LS identifies them as `name/arity` and `/` separates name path components. The arity is
|
||||
now separated by `#` instead (e.g. `create_user#4`), so the reported name path round-trips and
|
||||
`find_referencing_symbols`/`replace_symbol_body`/`insert_after_symbol` work on Erlang
|
||||
functions #1797
|
||||
|
||||
* JetBrains:
|
||||
- `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file
|
||||
|
||||
@@ -68,7 +68,8 @@ Some languages require additional installations or setup steps, as noted.
|
||||
* **Elm**
|
||||
(requires Elm compiler)
|
||||
* **Erlang**
|
||||
(requires installation of beam and [erlang_ls](https://github.com/erlang-ls/erlang_ls); experimental, might be slow or hang)
|
||||
(requires installation of beam and [erlang_ls](https://github.com/erlang-ls/erlang_ls); experimental, might be slow or hang;
|
||||
note that functions are addressed as `name#arity`, e.g. `create_user#4`, because `/` is reserved as the name path separator)
|
||||
* **F#**
|
||||
(requires [.NET v8.0+](https://dotnet.microsoft.com/en-us/download/dotnet); uses FsAutoComplete/Ionide, which is auto-installed; for Homebrew .NET on macOS, set DOTNET_ROOT in your environment)
|
||||
* **Fortran**
|
||||
|
||||
@@ -6,10 +6,11 @@ import shutil
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from collections.abc import Hashable
|
||||
|
||||
from overrides import override
|
||||
|
||||
from solidlsp.ls import SolidLanguageServer
|
||||
from solidlsp.ls import RawDocumentSymbol, SolidLanguageServer
|
||||
from solidlsp.ls_config import LanguageServerConfig
|
||||
from solidlsp.ls_utils import is_running_in_ci
|
||||
from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
|
||||
@@ -18,6 +19,14 @@ from solidlsp.util.subprocess_util import subprocess_run
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
ARITY_SEPARATOR = "#"
|
||||
"""
|
||||
The character that replaces the `/` in the `name/arity` identifiers reported by Erlang LS.
|
||||
|
||||
`#` was chosen because it cannot occur in an unquoted Erlang atom, so it can never collide with a
|
||||
real function, type or macro name (unlike `@`, which is a legal atom character).
|
||||
"""
|
||||
|
||||
|
||||
class ErlangLanguageServer(SolidLanguageServer):
|
||||
"""Language server for Erlang using Erlang LS."""
|
||||
@@ -48,6 +57,26 @@ class ErlangLanguageServer(SolidLanguageServer):
|
||||
# Set generous timeout for Erlang LS initialization
|
||||
self.set_request_timeout(120.0)
|
||||
|
||||
@override
|
||||
def _document_symbols_cache_fingerprint(self) -> Hashable:
|
||||
normalize_symbol_name_version = 1
|
||||
return normalize_symbol_name_version
|
||||
|
||||
@override
|
||||
def _normalize_symbol_name(self, symbol: RawDocumentSymbol, relative_file_path: str) -> str:
|
||||
"""
|
||||
Replaces the `/` in Erlang's `name/arity` identifiers, which would otherwise be interpreted
|
||||
as Serena's name path separator.
|
||||
|
||||
Erlang LS names functions, types and parameterised macros `name/arity` (e.g. `create_user/2`).
|
||||
Since `/` separates name path components, such a name is parsed as "symbol `2` nested inside
|
||||
`create_user`" and can never be matched, not even by the very name path that Serena itself
|
||||
reports for the symbol. The arity is not simply dropped because it is part of a function's
|
||||
identity in Erlang: `create_user/2` and `create_user/3` are different functions which may
|
||||
both be defined in the same module.
|
||||
"""
|
||||
return symbol["name"].replace("/", ARITY_SEPARATOR)
|
||||
|
||||
def _check_erlang_installation(self) -> bool:
|
||||
"""Check if Erlang/OTP is available."""
|
||||
try:
|
||||
|
||||
@@ -1872,6 +1872,12 @@ class SolidLanguageServer(ABC):
|
||||
NOTE: When changing the override of this method after the initial LS implementation,
|
||||
be sure to also override `_document_symbols_cache_fingerprint` in order to ensure that
|
||||
the caches are invalidated appropriately.
|
||||
NOTE: The returned name must not contain '/', which separates the components of a name path
|
||||
(see :class:`serena.symbol.NamePathMatcher`). A symbol whose name contains it cannot be
|
||||
addressed by any tool taking an exact name path, not even via the name path that is
|
||||
reported for the symbol itself. Language servers that identify symbols in such a way
|
||||
(e.g. Erlang LS, which reports functions as `name/arity`) must substitute another
|
||||
character here.
|
||||
|
||||
:param symbol: the symbol
|
||||
:param relative_file_path: the relative path of the file the symbol is located in
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
"""Normalization of Erlang's ``name/arity`` symbol identifiers.
|
||||
|
||||
Reproduces https://github.com/oraios/serena/issues/1797: Erlang LS names functions, types and
|
||||
parameterised macros ``name/arity`` (e.g. ``create_user/4``), but ``/`` separates the components of
|
||||
a Serena name path. Such a name was therefore parsed as "symbol ``4`` nested inside ``create_user``"
|
||||
and could never be matched -- not even by the very name path Serena itself reported for the symbol.
|
||||
In practice that made ``find_referencing_symbols``, ``replace_symbol_body`` and
|
||||
``insert_after_symbol`` unusable on Erlang functions, while read-only browsing kept working, so the
|
||||
language looked supported until one tried to do anything with a function.
|
||||
|
||||
``ErlangLanguageServer._normalize_symbol_name`` now substitutes
|
||||
:data:`~solidlsp.language_servers.erlang_language_server.ARITY_SEPARATOR` for the ``/``, which makes
|
||||
the reported name path round-trip back into the symbol tools.
|
||||
"""
|
||||
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from serena.project import Project
|
||||
from serena.symbol import LanguageServerSymbolRetriever
|
||||
from solidlsp.language_servers.erlang_language_server import ARITY_SEPARATOR
|
||||
from solidlsp.ls_config import LanguageServerId
|
||||
from solidlsp.ls_types import SymbolKind, UnifiedSymbolInformation
|
||||
from test.conftest import language_server_tests_enabled
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.erlang,
|
||||
pytest.mark.skipif(not language_server_tests_enabled(LanguageServerId.ERLANG), reason="Erlang tests are disabled"),
|
||||
]
|
||||
|
||||
MODELS_ERL = os.path.join("src", "models.erl")
|
||||
SERVICES_ERL = os.path.join("src", "services.erl")
|
||||
RECORDS_HRL = os.path.join("include", "records.hrl")
|
||||
|
||||
# `models:create_user/4`, spelled the way Serena addresses it
|
||||
CREATE_USER_4 = f"create_user{ARITY_SEPARATOR}4"
|
||||
|
||||
# file and directory symbols are named after path components, so `/` is legitimate for them
|
||||
CONTAINER_KINDS = (SymbolKind.File, SymbolKind.Package)
|
||||
|
||||
|
||||
class TestErlangSymbolNames:
|
||||
@pytest.mark.parametrize("project_with_ls", [LanguageServerId.ERLANG], indirect=True)
|
||||
def test_no_symbol_name_contains_the_name_path_separator(self, project_with_ls: Project) -> None:
|
||||
"""No Erlang symbol may carry a `/` in its name, since that is the name path separator."""
|
||||
offenders: list[str] = []
|
||||
|
||||
def visit(symbol: UnifiedSymbolInformation) -> None:
|
||||
if symbol["kind"] not in CONTAINER_KINDS and "/" in symbol["name"]:
|
||||
offenders.append(symbol["name"])
|
||||
for child in symbol.get("children", []):
|
||||
visit(child)
|
||||
|
||||
for ls in project_with_ls.get_language_server_manager_or_raise().iter_language_servers():
|
||||
for root in ls.request_full_symbol_tree():
|
||||
visit(root)
|
||||
|
||||
assert not offenders, f"Symbol names containing the name path separator: {sorted(set(offenders))}"
|
||||
|
||||
@pytest.mark.parametrize("project_with_ls", [LanguageServerId.ERLANG], indirect=True)
|
||||
def test_function_name_path_round_trips(self, project_with_ls: Project) -> None:
|
||||
"""The name path reported for a function must find that same function again."""
|
||||
retriever = LanguageServerSymbolRetriever(project_with_ls)
|
||||
|
||||
symbol = retriever.find_unique(CREATE_USER_4, within_relative_path=MODELS_ERL)
|
||||
assert symbol.symbol_kind == SymbolKind.Function
|
||||
assert symbol.get_name_path() == CREATE_USER_4
|
||||
|
||||
# feeding the reported name path back in is what used to yield no match at all
|
||||
assert [s.get_name_path() for s in retriever.find(symbol.get_name_path())] == [CREATE_USER_4]
|
||||
|
||||
@pytest.mark.parametrize("project_with_ls", [LanguageServerId.ERLANG], indirect=True)
|
||||
def test_arity_remains_part_of_the_name(self, project_with_ls: Project) -> None:
|
||||
"""The arity is kept rather than stripped: it is part of a function's identity in Erlang."""
|
||||
retriever = LanguageServerSymbolRetriever(project_with_ls)
|
||||
|
||||
# models:create_order/3 and services:create_order/2 are different functions
|
||||
models_create_order = retriever.find_unique(f"create_order{ARITY_SEPARATOR}3")
|
||||
services_create_order = retriever.find_unique(f"create_order{ARITY_SEPARATOR}2")
|
||||
|
||||
assert models_create_order.relative_path is not None
|
||||
assert models_create_order.relative_path.replace("\\", "/") == "src/models.erl"
|
||||
assert services_create_order.relative_path is not None
|
||||
assert services_create_order.relative_path.replace("\\", "/") == "src/services.erl"
|
||||
|
||||
@pytest.mark.parametrize("project_with_ls", [LanguageServerId.ERLANG], indirect=True)
|
||||
def test_find_referencing_symbols_locates_a_function(self, project_with_ls: Project) -> None:
|
||||
"""The headline symptom of #1797: this used to raise `No symbol matching ...`."""
|
||||
retriever = LanguageServerSymbolRetriever(project_with_ls)
|
||||
|
||||
references = retriever.find_referencing_symbols(CREATE_USER_4, MODELS_ERL)
|
||||
|
||||
# create_user/4 is called from models.erl itself and from at least one other module
|
||||
referencing_paths = {r.symbol.relative_path.replace("\\", "/") for r in references if r.symbol.relative_path is not None}
|
||||
assert referencing_paths, f"Expected references to {CREATE_USER_4}, got none"
|
||||
assert referencing_paths - {"src/models.erl"}, f"Expected cross-file references, got only {referencing_paths}"
|
||||
|
||||
@pytest.mark.parametrize("project_with_ls", [LanguageServerId.ERLANG], indirect=True)
|
||||
def test_names_without_an_arity_are_untouched(self, project_with_ls: Project) -> None:
|
||||
"""Records and macros have no arity, so normalization must leave their names alone."""
|
||||
retriever = LanguageServerSymbolRetriever(project_with_ls)
|
||||
|
||||
user_record = retriever.find_unique("user", within_relative_path=RECORDS_HRL)
|
||||
assert user_record.name == "user"
|
||||
Reference in new issue
Block a user