From 29d07d4f6b7a04a0db3981d6c6be6f736cfb44d2 Mon Sep 17 00:00:00 2001 From: Ivan Date: Sat, 1 Aug 2026 19:32:27 +0300 Subject: [PATCH] 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. --- CHANGELOG.md | 5 + docs/01-about/020_programming-languages.md | 3 +- .../erlang_language_server.py | 31 +++++- src/solidlsp/ls.py | 6 + .../erlang/test_erlang_symbol_names.py | 105 ++++++++++++++++++ 5 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 test/solidlsp/erlang/test_erlang_symbol_names.py diff --git a/CHANGELOG.md b/CHANGELOG.md index cf014908..94e06e64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,11 @@ Status of the `main` branch. Changes prior to the next official version change w - Fix: F#'s `module ` declarations reported a `selectionRange` pointing at the `module` keyword instead of at ``, 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 diff --git a/docs/01-about/020_programming-languages.md b/docs/01-about/020_programming-languages.md index 46d764db..00bfe33b 100644 --- a/docs/01-about/020_programming-languages.md +++ b/docs/01-about/020_programming-languages.md @@ -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** diff --git a/src/solidlsp/language_servers/erlang_language_server.py b/src/solidlsp/language_servers/erlang_language_server.py index c950aab1..f34c93f0 100644 --- a/src/solidlsp/language_servers/erlang_language_server.py +++ b/src/solidlsp/language_servers/erlang_language_server.py @@ -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: diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 1d117dba..a52ee0b1 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -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 diff --git a/test/solidlsp/erlang/test_erlang_symbol_names.py b/test/solidlsp/erlang/test_erlang_symbol_names.py new file mode 100644 index 00000000..e940fb60 --- /dev/null +++ b/test/solidlsp/erlang/test_erlang_symbol_names.py @@ -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"