mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-14 04:16:34 +00:00
simplify bash LS integration by removing hybrid approach (#467)
Turns out that bash language server was actually doing the right thing
This commit is contained in:
2 files changed
+37
-219
No files matched your search
@@ -198,185 +198,33 @@ class BashLanguageServer(SolidLanguageServer):
|
||||
self, relative_file_path: str, include_body: bool = False
|
||||
) -> tuple[list[ls_types.UnifiedSymbolInformation], list[ls_types.UnifiedSymbolInformation]]:
|
||||
"""
|
||||
Enhanced document symbol request with hybrid LSP + regex-based function detection for bash files.
|
||||
Request document symbols from bash-language-server via LSP.
|
||||
|
||||
This method combines both LSP-based detection and regex-based detection to provide comprehensive
|
||||
function discovery. This dual approach is necessary because:
|
||||
Uses the standard LSP documentSymbol request which provides reliable function detection
|
||||
for all bash function syntaxes including:
|
||||
- function name() { ... } (with function keyword)
|
||||
- name() { ... } (traditional syntax)
|
||||
- Functions with various indentation levels
|
||||
- Functions with comments before/after/inside
|
||||
|
||||
1. bash-language-server (v5.6.0) has inconsistent function detection capabilities
|
||||
2. Some bash function syntaxes are not reliably detected by the LSP server
|
||||
3. Files may contain mixed function notation styles within the same file
|
||||
4. Different formatting or indentation can affect LSP detection
|
||||
Args:
|
||||
relative_file_path: Path to the bash file relative to repository root
|
||||
include_body: Whether to include function bodies in symbol information
|
||||
|
||||
Returns:
|
||||
Tuple of (all_symbols, root_symbols) detected by the LSP server
|
||||
|
||||
The hybrid approach ensures maximum compatibility and comprehensive function discovery
|
||||
for reliable symbolic editing operations in Serena.
|
||||
"""
|
||||
# First get symbols from the standard LSP approach
|
||||
lsp_all_symbols, lsp_root_symbols = super().request_document_symbols(relative_file_path, include_body)
|
||||
self.logger.log(f"Requesting document symbols via LSP for {relative_file_path}", logging.DEBUG)
|
||||
|
||||
# Always run regex-based function detection for comprehensive coverage
|
||||
# This addresses bash-language-server limitations and ensures we catch all function patterns
|
||||
self.logger.log(f"Running hybrid function detection (LSP + regex) for {relative_file_path}", logging.DEBUG)
|
||||
|
||||
regex_detected_functions = self._detect_bash_functions(relative_file_path, include_body)
|
||||
|
||||
# Merge LSP and regex results, avoiding duplicates
|
||||
merged_all_symbols, merged_root_symbols = self._merge_function_detections(
|
||||
lsp_all_symbols, lsp_root_symbols, regex_detected_functions
|
||||
)
|
||||
# Use the standard LSP approach - bash-language-server handles all function syntaxes correctly
|
||||
all_symbols, root_symbols = super().request_document_symbols(relative_file_path, include_body)
|
||||
|
||||
# Log detection results for debugging
|
||||
lsp_functions = [s for s in lsp_all_symbols if s.get("kind") == 12]
|
||||
total_functions = [s for s in merged_all_symbols if s.get("kind") == 12]
|
||||
|
||||
functions = [s for s in all_symbols if s.get("kind") == 12]
|
||||
self.logger.log(
|
||||
f"Function detection for {relative_file_path}: LSP={len(lsp_functions)}, "
|
||||
f"Regex={len(regex_detected_functions)}, Total={len(total_functions)}",
|
||||
f"LSP function detection for {relative_file_path}: Found {len(functions)} functions",
|
||||
logging.INFO,
|
||||
)
|
||||
|
||||
return merged_all_symbols, merged_root_symbols
|
||||
|
||||
def _merge_function_detections(
|
||||
self,
|
||||
lsp_all_symbols: list[ls_types.UnifiedSymbolInformation],
|
||||
lsp_root_symbols: list[ls_types.UnifiedSymbolInformation],
|
||||
regex_detected_functions: list[ls_types.UnifiedSymbolInformation],
|
||||
) -> tuple[list[ls_types.UnifiedSymbolInformation], list[ls_types.UnifiedSymbolInformation]]:
|
||||
"""
|
||||
Merge LSP-detected symbols with regex-detected functions, avoiding duplicates.
|
||||
|
||||
This method:
|
||||
1. Keeps all non-function symbols from LSP detection
|
||||
2. Keeps all LSP-detected functions (they have more accurate positioning)
|
||||
3. Adds regex-detected functions that weren't found by LSP
|
||||
4. Uses function names to detect duplicates
|
||||
|
||||
Args:
|
||||
lsp_all_symbols: All symbols detected by LSP
|
||||
lsp_root_symbols: Root-level symbols detected by LSP
|
||||
regex_detected_functions: Functions detected by regex (all are root-level)
|
||||
|
||||
Returns:
|
||||
Tuple of (merged_all_symbols, merged_root_symbols)
|
||||
|
||||
"""
|
||||
# Extract function names that LSP already detected
|
||||
lsp_function_names = set()
|
||||
for symbol in lsp_all_symbols:
|
||||
if symbol.get("kind") == 12: # LSP Symbol Kind 12 = Function
|
||||
lsp_function_names.add(symbol["name"])
|
||||
|
||||
# Start with all LSP symbols (both functions and non-functions)
|
||||
merged_all_symbols = lsp_all_symbols.copy()
|
||||
merged_root_symbols = lsp_root_symbols.copy()
|
||||
|
||||
# Add regex-detected functions that weren't found by LSP
|
||||
added_functions = 0
|
||||
for regex_function in regex_detected_functions:
|
||||
function_name = regex_function["name"]
|
||||
|
||||
# Only add if this function wasn't detected by LSP
|
||||
if function_name not in lsp_function_names:
|
||||
merged_all_symbols.append(regex_function)
|
||||
merged_root_symbols.append(regex_function) # All regex functions are root-level
|
||||
added_functions += 1
|
||||
self.logger.log(f"Added regex-detected function '{function_name}' not found by LSP", logging.DEBUG)
|
||||
|
||||
if added_functions > 0:
|
||||
self.logger.log(f"Merged {added_functions} additional functions from regex detection", logging.INFO)
|
||||
|
||||
return merged_all_symbols, merged_root_symbols
|
||||
|
||||
def _detect_bash_functions(self, relative_file_path: str, include_body: bool = False) -> list[ls_types.UnifiedSymbolInformation]:
|
||||
"""
|
||||
Regex-based detection of bash functions as fallback when LSP doesn't provide them.
|
||||
"""
|
||||
import re
|
||||
|
||||
from solidlsp import ls_types
|
||||
|
||||
try:
|
||||
# Read the file content directly from filesystem
|
||||
abs_path = os.path.join(self.repository_root_path, relative_file_path)
|
||||
with open(abs_path, encoding="utf-8") as f:
|
||||
file_content = f.read()
|
||||
lines = file_content.split("\n")
|
||||
|
||||
detected_functions = []
|
||||
|
||||
# Regex patterns for bash function definitions
|
||||
# Pattern 1: function name() { ... }
|
||||
# Pattern 2: name() { ... }
|
||||
function_patterns = [
|
||||
re.compile(r"^\s*function\s+([a-zA-Z_][a-zA-Z0-9_]*)\s*\(\s*\)\s*\{"),
|
||||
re.compile(r"^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\(\s*\)\s*\{"),
|
||||
]
|
||||
|
||||
for line_num, line in enumerate(lines):
|
||||
for pattern in function_patterns:
|
||||
match = pattern.match(line)
|
||||
if match:
|
||||
func_name = match.group(1)
|
||||
|
||||
# Find the end of the function by matching braces
|
||||
start_line = line_num
|
||||
end_line = self._find_function_end(lines, start_line)
|
||||
|
||||
# Create location information
|
||||
location = ls_types.Location(
|
||||
uri=pathlib.Path(abs_path).as_uri(),
|
||||
range={"start": {"line": start_line, "character": match.start(1)}, "end": {"line": end_line, "character": 0}},
|
||||
absolutePath=abs_path,
|
||||
relativePath=relative_file_path,
|
||||
)
|
||||
|
||||
# Create the function symbol
|
||||
func_symbol = ls_types.UnifiedSymbolInformation(
|
||||
name=func_name,
|
||||
kind=12, # LSP Symbol Kind for Function
|
||||
location=location,
|
||||
range=location["range"],
|
||||
selectionRange={
|
||||
"start": {"line": start_line, "character": match.start(1)},
|
||||
"end": {"line": start_line, "character": match.end(1)},
|
||||
},
|
||||
children=[],
|
||||
parent=None,
|
||||
)
|
||||
|
||||
# Add function body if requested
|
||||
if include_body:
|
||||
func_symbol["body"] = "\n".join(lines[start_line : end_line + 1])
|
||||
|
||||
detected_functions.append(func_symbol)
|
||||
break
|
||||
|
||||
return detected_functions
|
||||
|
||||
except Exception as e:
|
||||
self.logger.log(f"Error in regex-based function detection for {relative_file_path}: {e}", logging.WARNING)
|
||||
return []
|
||||
|
||||
def _find_function_end(self, lines: list[str], start_line: int) -> int:
|
||||
"""
|
||||
Find the end line of a bash function by matching opening and closing braces.
|
||||
"""
|
||||
brace_count = 0
|
||||
in_function = False
|
||||
|
||||
for i in range(start_line, len(lines)):
|
||||
line = lines[i]
|
||||
|
||||
# Count braces, handling basic cases
|
||||
for char in line:
|
||||
if char == "{":
|
||||
brace_count += 1
|
||||
in_function = True
|
||||
elif char == "}" and in_function:
|
||||
brace_count -= 1
|
||||
if brace_count == 0:
|
||||
return i
|
||||
|
||||
# Fallback: if we can't match braces properly, assume single line or small function
|
||||
return min(start_line + 10, len(lines) - 1)
|
||||
return all_symbols, root_symbols
|
||||
@@ -80,62 +80,28 @@ class TestBashLanguageServerBasics:
|
||||
|
||||
assert len(utils_function_symbols) >= 8, f"Should find at least 8 functions in utils.sh, found {len(utils_function_symbols)}"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.BASH], indirect=True)
|
||||
def test_bash_regex_fallback_detection(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Test the regex-based fallback function detection directly."""
|
||||
# Check if the method exists (it should if we have our enhanced bash server)
|
||||
if not hasattr(language_server, "_detect_bash_functions"):
|
||||
pytest.fail("Enhanced bash function detection not available - this may be due to test environment setup")
|
||||
|
||||
# Test the _detect_bash_functions method directly
|
||||
detected_functions = language_server._detect_bash_functions("main.sh", include_body=True)
|
||||
|
||||
# Should detect functions even if LSP doesn't provide them
|
||||
assert len(detected_functions) == 3, f"Should detect exactly 3 functions, found {len(detected_functions)}"
|
||||
|
||||
function_names = [func["name"] for func in detected_functions]
|
||||
assert "greet_user" in function_names, "Regex should detect greet_user function"
|
||||
assert "process_items" in function_names, "Regex should detect process_items function"
|
||||
assert "main" in function_names, "Regex should detect main function"
|
||||
|
||||
# Check that symbols have proper structure
|
||||
for func in detected_functions:
|
||||
assert func["kind"] == 12, f"Function {func['name']} should have LSP Symbol Kind 12"
|
||||
assert "location" in func, f"Function {func['name']} should have location info"
|
||||
assert "range" in func, f"Function {func['name']} should have range info"
|
||||
assert "selectionRange" in func, f"Function {func['name']} should have selectionRange info"
|
||||
|
||||
# Check location structure
|
||||
location = func["location"]
|
||||
assert "uri" in location, f"Function {func['name']} location should have URI"
|
||||
assert "range" in location, f"Function {func['name']} location should have range"
|
||||
assert "relativePath" in location, f"Function {func['name']} location should have relative path"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.BASH], indirect=True)
|
||||
def test_bash_function_syntax_patterns(self, language_server: SolidLanguageServer) -> None:
|
||||
"""Test detection of different bash function syntax patterns."""
|
||||
# Check if the method exists (it should if we have our enhanced bash server)
|
||||
if not hasattr(language_server, "_detect_bash_functions"):
|
||||
pytest.fail("Enhanced bash function detection not available - this may be due to test environment setup")
|
||||
|
||||
# Test main.sh (has 'function' keyword functions and regular function)
|
||||
main_functions = language_server._detect_bash_functions("main.sh", include_body=False)
|
||||
"""Test that LSP detects different bash function syntax patterns correctly."""
|
||||
# Test main.sh (has both 'function' keyword and traditional syntax)
|
||||
main_all_symbols, main_root_symbols = language_server.request_document_symbols("main.sh", include_body=False)
|
||||
main_functions = [symbol for symbol in main_all_symbols if symbol.get("kind") == 12]
|
||||
main_function_names = [func["name"] for func in main_functions]
|
||||
|
||||
# Test utils.sh (all use 'function' keyword)
|
||||
utils_functions = language_server._detect_bash_functions("utils.sh", include_body=False)
|
||||
|
||||
# Verify we detect both syntax patterns
|
||||
main_function_names = [func["name"] for func in main_functions]
|
||||
utils_all_symbols, utils_root_symbols = language_server.request_document_symbols("utils.sh", include_body=False)
|
||||
utils_functions = [symbol for symbol in utils_all_symbols if symbol.get("kind") == 12]
|
||||
utils_function_names = [func["name"] for func in utils_functions]
|
||||
|
||||
# main() uses regular syntax: main() {
|
||||
assert "main" in main_function_names, "Should detect regular function syntax"
|
||||
# Verify LSP detects both syntax patterns
|
||||
# main() uses traditional syntax: main() {
|
||||
assert "main" in main_function_names, "LSP should detect traditional function syntax"
|
||||
|
||||
# Functions with 'function' keyword: function name() {
|
||||
assert "greet_user" in main_function_names, "Should detect function keyword syntax"
|
||||
assert "to_uppercase" in utils_function_names, "Should detect function keyword syntax in utils"
|
||||
assert "greet_user" in main_function_names, "LSP should detect function keyword syntax"
|
||||
assert "process_items" in main_function_names, "LSP should detect function keyword syntax"
|
||||
|
||||
# Verify all expected utils functions are detected
|
||||
# Verify all expected utils functions are detected by LSP
|
||||
expected_utils = [
|
||||
"to_uppercase",
|
||||
"to_lowercase",
|
||||
@@ -148,4 +114,8 @@ class TestBashLanguageServerBasics:
|
||||
]
|
||||
|
||||
for expected_func in expected_utils:
|
||||
assert expected_func in utils_function_names, f"Should detect {expected_func} function"
|
||||
assert expected_func in utils_function_names, f"LSP should detect {expected_func} function"
|
||||
|
||||
# Verify total counts match expectations
|
||||
assert len(main_functions) >= 3, f"Should find at least 3 functions in main.sh, found {len(main_functions)}"
|
||||
assert len(utils_functions) >= 8, f"Should find at least 8 functions in utils.sh, found {len(utils_functions)}"
|
||||
Reference in new issue
Block a user