mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-12 18:16:43 +00:00
Fix PHP symbol retrieval: declare missing Intelephense client capabilities (#1099)
Fix PHP symbol retrieval via Intelephense LSP * intelephense.py: Add missing client capability declarations for documentSymbol (with hierarchical support), references, hover, and workspace symbol. Without these, Intelephense does not advertise documentSymbolProvider and returns empty results for get_symbols_overview. Add PHP tests validating hierarchical symbols and LS routing fix - test_document_symbols_hierarchical_structure: verifies Intelephense returns DocumentSymbol[] with class/method hierarchy (not flat SymbolInformation[]) when hierarchicalDocumentSymbolSupport is declared - test_full_symbol_tree_within_file: verifies request_full_symbol_tree scoped to sample.php returns Dog/Animal/greet/fetch with Dog containing greet as a child (not at root level) - test_find_symbol_within_php_file: verifies find_symbol with a PHP file path (relative_path=) routes to the PHP language server via get_language_server() rather than iterating all servers Peripheral changes: - symbol.py (LanguageServerSymbolRetriever.find_symbols): When searching within a specific file, route to the file-type-appropriate language server via get_language_server() instead of querying all servers. This ensures PHP files are served by Intelephense and not silently skipped.
This commit is contained in:
1 parent
b3a6045f7e
commit
18cee10fa9
5 files changed
+246
-6
No files matched your search
+11
-2
@@ -2,7 +2,7 @@ import json
|
||||
import logging
|
||||
import os
|
||||
from abc import ABC, abstractmethod
|
||||
from collections.abc import Callable, Iterator, Sequence
|
||||
from collections.abc import Callable, Iterable, Iterator, Sequence
|
||||
from dataclasses import asdict, dataclass
|
||||
from time import perf_counter
|
||||
from typing import Any, Generic, Literal, NotRequired, Self, TypedDict, TypeVar
|
||||
@@ -701,7 +701,16 @@ class LanguageServerSymbolRetriever:
|
||||
optionally limited to a specific file and filtered by kind.
|
||||
"""
|
||||
symbols: list[LanguageServerSymbol] = []
|
||||
for lang_server in self._ls_manager.iter_language_servers():
|
||||
if within_relative_path and os.path.isfile(os.path.join(self.project.project_root, within_relative_path)):
|
||||
"""
|
||||
For a specific file, use get_language_server to select the best LS for the file type
|
||||
(consistent with get_symbol_overview). This ensures e.g. PHP files are served by the
|
||||
PHP language server rather than being rejected by all LSes via is_ignored_path.
|
||||
"""
|
||||
lang_servers: Iterable[SolidLanguageServer] = [self._ls_manager.get_language_server(within_relative_path)]
|
||||
else:
|
||||
lang_servers = self._ls_manager.iter_language_servers()
|
||||
for lang_server in lang_servers:
|
||||
symbol_roots = lang_server.request_full_symbol_tree(within_relative_path=within_relative_path)
|
||||
for root in symbol_roots:
|
||||
symbols.extend(
|
||||
|
||||
@@ -112,8 +112,19 @@ class Intelephense(SolidLanguageServer):
|
||||
"textDocument": {
|
||||
"synchronization": {"didSave": True, "dynamicRegistration": True},
|
||||
"definition": {"dynamicRegistration": True},
|
||||
"references": {"dynamicRegistration": True},
|
||||
"documentSymbol": {
|
||||
"dynamicRegistration": True,
|
||||
"hierarchicalDocumentSymbolSupport": True,
|
||||
"symbolKind": {"valueSet": list(range(1, 27))},
|
||||
},
|
||||
"hover": {"dynamicRegistration": True, "contentFormat": ["markdown", "plaintext"]},
|
||||
},
|
||||
"workspace": {
|
||||
"workspaceFolders": True,
|
||||
"didChangeConfiguration": {"dynamicRegistration": True},
|
||||
"symbol": {"dynamicRegistration": True},
|
||||
},
|
||||
"workspace": {"workspaceFolders": True, "didChangeConfiguration": {"dynamicRegistration": True}},
|
||||
},
|
||||
"processId": os.getpid(),
|
||||
"rootPath": repository_absolute_path,
|
||||
@@ -167,9 +178,11 @@ class Intelephense(SolidLanguageServer):
|
||||
log.info("After sent initialize params")
|
||||
|
||||
# Verify server capabilities
|
||||
assert "textDocumentSync" in init_response["capabilities"]
|
||||
assert "completionProvider" in init_response["capabilities"]
|
||||
assert "definitionProvider" in init_response["capabilities"]
|
||||
capabilities = init_response["capabilities"]
|
||||
assert "textDocumentSync" in capabilities
|
||||
assert "completionProvider" in capabilities
|
||||
assert "definitionProvider" in capabilities
|
||||
assert "documentSymbolProvider" in capabilities, "Server must support document symbols"
|
||||
|
||||
self.server.notify.initialized({})
|
||||
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
<?php
|
||||
|
||||
namespace SerenaTest;
|
||||
|
||||
/**
|
||||
* Interface for objects that can greet.
|
||||
*/
|
||||
interface GreeterInterface
|
||||
{
|
||||
public function greet(string $name): string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Abstract base class for animals.
|
||||
*/
|
||||
abstract class Animal
|
||||
{
|
||||
protected string $name;
|
||||
protected int $age;
|
||||
|
||||
public function __construct(string $name, int $age)
|
||||
{
|
||||
$this->name = $name;
|
||||
$this->age = $age;
|
||||
}
|
||||
|
||||
public function getName(): string
|
||||
{
|
||||
return $this->name;
|
||||
}
|
||||
|
||||
public function getAge(): int
|
||||
{
|
||||
return $this->age;
|
||||
}
|
||||
|
||||
abstract public function describe(): string;
|
||||
}
|
||||
|
||||
/**
|
||||
* A concrete animal that can greet visitors.
|
||||
*/
|
||||
class Dog extends Animal implements GreeterInterface
|
||||
{
|
||||
private string $breed;
|
||||
|
||||
public function __construct(string $name, int $age, string $breed)
|
||||
{
|
||||
parent::__construct($name, $age);
|
||||
$this->breed = $breed;
|
||||
}
|
||||
|
||||
public function greet(string $visitorName): string
|
||||
{
|
||||
return "Woof! I'm {$this->name}. Hello, {$visitorName}!";
|
||||
}
|
||||
|
||||
public function getBreed(): string
|
||||
{
|
||||
return $this->breed;
|
||||
}
|
||||
|
||||
public function describe(): string
|
||||
{
|
||||
return "Dog: {$this->name} ({$this->breed}), age {$this->age}";
|
||||
}
|
||||
|
||||
public function fetch(string $item): string
|
||||
{
|
||||
return "{$this->name} fetches the {$item}!";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Another concrete animal.
|
||||
*/
|
||||
class Cat extends Animal
|
||||
{
|
||||
private bool $indoor;
|
||||
|
||||
public function __construct(string $name, int $age, bool $indoor = true)
|
||||
{
|
||||
parent::__construct($name, $age);
|
||||
$this->indoor = $indoor;
|
||||
}
|
||||
|
||||
public function isIndoor(): bool
|
||||
{
|
||||
return $this->indoor;
|
||||
}
|
||||
|
||||
public function describe(): string
|
||||
{
|
||||
$type = $this->indoor ? 'indoor' : 'outdoor';
|
||||
return "Cat: {$this->name} ({$type}), age {$this->age}";
|
||||
}
|
||||
}
|
||||
|
||||
const MAX_ANIMALS = 100;
|
||||
const DEFAULT_BREED = 'Mixed';
|
||||
|
||||
/**
|
||||
* Factory function to create an animal by type name.
|
||||
*/
|
||||
function createAnimal(string $type, string $name, int $age): Animal
|
||||
{
|
||||
return match ($type) {
|
||||
'dog' => new Dog($name, $age, DEFAULT_BREED),
|
||||
'cat' => new Cat($name, $age),
|
||||
default => throw new \InvalidArgumentException("Unknown animal type: {$type}"),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a summary string for a list of animals.
|
||||
*
|
||||
* @param Animal[] $animals
|
||||
*/
|
||||
function summarizeAnimals(array $animals): string
|
||||
{
|
||||
return implode(', ', array_map(fn(Animal $a) => $a->describe(), $animals));
|
||||
}
|
||||
@@ -146,6 +146,30 @@ class TestSerenaAgent:
|
||||
if s["kind"] == SymbolKind.Class.name and serena_agent.get_active_lsp_languages() == [Language.JAVA]:
|
||||
assert "A simple model class" in symbol_info, f"Java class docstring not found in symbol info: {s}"
|
||||
|
||||
@pytest.mark.php
|
||||
@pytest.mark.parametrize("serena_agent", [Language.PHP], indirect=True)
|
||||
def test_find_symbol_within_php_file(self, serena_agent: SerenaAgent) -> None:
|
||||
"""Verify find_symbol with a PHP file path routes to the PHP language server.
|
||||
|
||||
This validates the fix in symbol.py (LanguageServerSymbolRetriever.find_symbols):
|
||||
when within_relative_path points to a PHP file, the retriever must use
|
||||
get_language_server() rather than iterating all language servers. Without this
|
||||
fix, non-PHP servers reject the PHP file and no symbols are returned.
|
||||
"""
|
||||
find_symbol_tool = serena_agent.get_tool(FindSymbolTool)
|
||||
sample_php = "sample.php"
|
||||
|
||||
result = find_symbol_tool.apply(name_path_pattern="Dog/greet", relative_path=sample_php)
|
||||
symbols = json.loads(result)
|
||||
|
||||
assert len(symbols) > 0, (
|
||||
f"Expected to find Dog/greet in {sample_php} but got empty result. "
|
||||
"This may indicate that find_symbol is not routing to the PHP language server for PHP files."
|
||||
)
|
||||
assert any(
|
||||
"greet" in s["name_path"] and sample_php in s["relative_path"] for s in symbols
|
||||
), f"Dog/greet not found in {sample_php}. Symbols: {symbols}"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"serena_agent,symbol_name,expected_kind,expected_file",
|
||||
[
|
||||
|
||||
@@ -184,3 +184,75 @@ class TestPhpLanguageServers:
|
||||
|
||||
usage_in_index_php = {"uri_suffix": "index.php", "line": 13, "character": 0}
|
||||
assert usage_in_index_php in actual_locations_comparable, "Usage of helperFunction in index.php not found"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
|
||||
def test_find_symbol(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Test that document symbols are properly retrieved after Intelephense capability fix."""
|
||||
from solidlsp.ls_utils import SymbolUtils
|
||||
|
||||
symbols = language_server.request_full_symbol_tree()
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "helperFunction"), "helperFunction not found in symbol tree"
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "greet"), "greet function not found in symbol tree"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
|
||||
def test_document_symbols(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Test that document symbols are properly retrieved for a specific file."""
|
||||
doc_symbols = language_server.request_document_symbols("helper.php")
|
||||
all_symbols = doc_symbols.get_all_symbols_and_roots()
|
||||
symbol_names = [sym.get("name") for sym in all_symbols[0] if sym.get("name")]
|
||||
assert "helperFunction" in symbol_names, f"helperFunction not found in document symbols. Found: {symbol_names}"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
|
||||
def test_document_symbols_hierarchical_structure(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Verify Intelephense returns hierarchical DocumentSymbol format.
|
||||
|
||||
When hierarchicalDocumentSymbolSupport is declared in client capabilities,
|
||||
Intelephense returns DocumentSymbol[] where class methods appear as children
|
||||
of their parent class. Without this declaration, it falls back to a flat
|
||||
SymbolInformation[] list where all symbols appear at root level with no
|
||||
parent-child relationships.
|
||||
"""
|
||||
all_symbols, root_symbols = language_server.request_document_symbols("sample.php").get_all_symbols_and_roots()
|
||||
|
||||
root_names = [s.get("name") for s in root_symbols]
|
||||
assert "Animal" in root_names, f"Animal class not found at root level. Roots: {root_names}"
|
||||
assert "Dog" in root_names, f"Dog class not found at root level. Roots: {root_names}"
|
||||
assert "Cat" in root_names, f"Cat class not found at root level. Roots: {root_names}"
|
||||
|
||||
# Verify Dog has method children — this is the key assertion for hierarchical support.
|
||||
# With a flat response, Dog would have no children and all methods would be at root level.
|
||||
dog_symbol = next((s for s in root_symbols if s.get("name") == "Dog"), None)
|
||||
assert dog_symbol is not None, "Dog class not found in root symbols"
|
||||
dog_children = dog_symbol.get("children", [])
|
||||
dog_child_names = [c.get("name") for c in dog_children]
|
||||
assert (
|
||||
len(dog_child_names) > 0
|
||||
), f"Dog class has no children — hierarchicalDocumentSymbolSupport is not working. All root symbols: {root_names}"
|
||||
expected_methods = {"greet", "fetch", "getBreed", "describe"}
|
||||
missing = expected_methods - set(dog_child_names)
|
||||
assert not missing, f"Dog class missing expected methods: {missing}. Children found: {dog_child_names}"
|
||||
|
||||
# Methods must NOT appear at root level (that would indicate the flat fallback format).
|
||||
assert "greet" not in root_names, f"greet should be a child of Dog, not at root level. Roots: {root_names}"
|
||||
assert "fetch" not in root_names, f"fetch should be a child of Dog, not at root level. Roots: {root_names}"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
|
||||
def test_full_symbol_tree_within_file(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Verify request_full_symbol_tree scoped to a PHP file returns correct symbols.
|
||||
|
||||
This validates that Intelephense responds correctly when symbols are requested
|
||||
for a single file, including class/method hierarchy in sample.php.
|
||||
"""
|
||||
from solidlsp.ls_utils import SymbolUtils
|
||||
|
||||
symbols = language_server.request_full_symbol_tree(within_relative_path="sample.php")
|
||||
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "Dog"), "Dog not found in sample.php symbol tree"
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "Animal"), "Animal not found in sample.php symbol tree"
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "greet"), "greet method not found in sample.php symbol tree"
|
||||
assert SymbolUtils.symbol_tree_contains_name(symbols, "fetch"), "fetch method not found in sample.php symbol tree"
|
||||
|
||||
# Methods must appear as children of Dog, not as root-level symbols
|
||||
dog_root = next((s for s in symbols if s.get("name") == "Dog"), None)
|
||||
if dog_root is not None:
|
||||
assert SymbolUtils.symbol_tree_contains_name([dog_root], "greet"), "greet should be nested under Dog in symbol tree"
|
||||
Reference in new issue
Block a user