mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 08:15:59 +00:00
fix: detect rust-analyzer from PATH without rustup (#1173)
* fix: check PATH for rust-analyzer before common locations, with functionality validation Fixes #800 - rust-analyzer fails if rustup is not installed even when rust-analyzer exists in PATH (e.g. NixOS, distro packages). Changes: - Move PATH check (shutil.which) before common location fallbacks - Add _is_rust_analyzer_functional() to validate PATH-found binaries by running --version, catching broken rustup proxy symlinks - Add tests for Nix PATH detection and broken proxy skip scenarios * fix: make test_skip_broken_rustup_proxy_in_path work on Windows The test used hardcoded Unix paths (/usr/bin/rust-analyzer) and os.path.expanduser("~/.cargo/bin/rust-analyzer") which doesn't include the .exe extension. On Windows, the production code builds common paths using pathlib with "rust-analyzer.exe", so the mock paths never matched, causing RuntimeError instead of finding the cargo fallback. Use platform-appropriate paths: on Windows, use .exe extension and pathlib.Path.home() to match how the production code constructs paths. --------- Co-authored-by: VoidChecksum <Admin@CyberNord>
This commit is contained in:
1 parent
2ccd396d74
commit
d39d168523
2 files changed
+107
-26
No files matched your search
@@ -65,6 +65,19 @@ class RustAnalyzer(SolidLanguageServer):
|
||||
pass
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _is_rust_analyzer_functional(path: str) -> bool:
|
||||
"""Check if a rust-analyzer binary is functional by running --version.
|
||||
|
||||
This catches the case where rust-analyzer in PATH is actually a rustup proxy
|
||||
that fails because the component is not installed.
|
||||
"""
|
||||
try:
|
||||
result = subprocess.run([path, "--version"], capture_output=True, text=True, check=False, timeout=10)
|
||||
return result.returncode == 0
|
||||
except (FileNotFoundError, OSError, subprocess.TimeoutExpired):
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def _ensure_rust_analyzer_installed() -> str:
|
||||
"""
|
||||
@@ -73,8 +86,8 @@ class RustAnalyzer(SolidLanguageServer):
|
||||
Priority order:
|
||||
1. Rustup existing installation (preferred - matches toolchain version)
|
||||
2. Rustup auto-install if rustup is available (ensures correct version)
|
||||
3. Common installation locations as fallback (only if rustup not available)
|
||||
4. System PATH last (can pick up incompatible versions)
|
||||
3. System PATH (covers Nix, distro packages, and other standalone installs)
|
||||
4. Common installation locations as final fallback
|
||||
|
||||
:return: path to rust-analyzer executable
|
||||
"""
|
||||
@@ -84,7 +97,7 @@ class RustAnalyzer(SolidLanguageServer):
|
||||
return rustup_path
|
||||
|
||||
# If rustup is available but rust-analyzer not installed, auto-install it BEFORE
|
||||
# checking common paths. This ensures we get the correct version matching the toolchain.
|
||||
# checking PATH. This ensures we get the correct version matching the toolchain.
|
||||
if RustAnalyzer.DependencyProvider._get_rustup_version():
|
||||
result = subprocess.run(["rustup", "component", "add", "rust-analyzer"], check=False, capture_output=True, text=True)
|
||||
if result.returncode == 0:
|
||||
@@ -92,13 +105,20 @@ class RustAnalyzer(SolidLanguageServer):
|
||||
rustup_path = RustAnalyzer.DependencyProvider._get_rust_analyzer_via_rustup()
|
||||
if rustup_path:
|
||||
return rustup_path
|
||||
# If auto-install failed, fall through to common paths as last resort
|
||||
# If auto-install failed, fall through to PATH and common paths
|
||||
|
||||
# Check system PATH early - this covers Nix, distro packages, and other standalone installs.
|
||||
# We verify the binary is functional to avoid picking up broken rustup proxy symlinks.
|
||||
path_result = shutil.which("rust-analyzer")
|
||||
if path_result and os.path.isfile(path_result) and os.access(path_result, os.X_OK):
|
||||
if RustAnalyzer.DependencyProvider._is_rust_analyzer_functional(path_result):
|
||||
return path_result
|
||||
|
||||
# Determine platform-specific binary name and paths
|
||||
is_windows = platform.system() == "Windows"
|
||||
binary_name = "rust-analyzer.exe" if is_windows else "rust-analyzer"
|
||||
|
||||
# Fallback to common installation locations (only used if rustup not available)
|
||||
# Fallback to common installation locations
|
||||
common_paths: list[str | None] = []
|
||||
|
||||
if is_windows:
|
||||
@@ -129,11 +149,6 @@ class RustAnalyzer(SolidLanguageServer):
|
||||
if path and os.path.isfile(path) and os.access(path, os.X_OK):
|
||||
return path
|
||||
|
||||
# Last resort: check system PATH (can pick up incorrect aliases, hence checked last)
|
||||
path_result = shutil.which("rust-analyzer")
|
||||
if path_result and os.path.isfile(path_result) and os.access(path_result, os.X_OK):
|
||||
return path_result
|
||||
|
||||
# Provide helpful error message with all searched locations
|
||||
searched = [p for p in common_paths if p]
|
||||
install_instructions = [
|
||||
|
||||
@@ -33,38 +33,104 @@ class TestRustAnalyzerDetection:
|
||||
"""Unit tests for rust-analyzer binary detection logic."""
|
||||
|
||||
@pytest.mark.rust
|
||||
def test_detect_from_path_as_last_resort(self):
|
||||
def test_detect_from_path_when_functional(self):
|
||||
"""
|
||||
GIVEN rustup is not available
|
||||
AND rust-analyzer is NOT in common locations (Homebrew, cargo)
|
||||
AND rust-analyzer IS in system PATH
|
||||
AND rust-analyzer IS in system PATH and is functional
|
||||
WHEN _ensure_rust_analyzer_installed is called
|
||||
THEN it should return the path from shutil.which as last resort
|
||||
THEN it should return the path from shutil.which
|
||||
|
||||
WHY: PATH is checked last to avoid picking up incorrect aliases.
|
||||
Users with rust-analyzer in PATH but not via rustup/common locations
|
||||
should still work.
|
||||
WHY: PATH is checked after rustup but before common locations.
|
||||
This covers standalone installs via Nix, distro packages, etc.
|
||||
The binary is verified to be functional to avoid broken rustup proxy symlinks.
|
||||
Fixes: https://github.com/oraios/serena/issues/800
|
||||
"""
|
||||
from solidlsp.language_servers.rust_analyzer import RustAnalyzer
|
||||
|
||||
# Mock rustup to be unavailable
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rust_analyzer_via_rustup", return_value=None):
|
||||
# Mock common locations to NOT exist
|
||||
with patch("os.path.isfile", return_value=False):
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rustup_version", return_value=None):
|
||||
# Mock PATH to have rust-analyzer
|
||||
with patch("shutil.which") as mock_which:
|
||||
mock_which.return_value = "/custom/bin/rust-analyzer"
|
||||
with patch("os.access", return_value=True):
|
||||
# Need isfile to return True for PATH result only
|
||||
def selective_isfile(path):
|
||||
return path == "/custom/bin/rust-analyzer"
|
||||
|
||||
with patch("os.path.isfile", side_effect=selective_isfile):
|
||||
result = RustAnalyzer.DependencyProvider._ensure_rust_analyzer_installed()
|
||||
with patch("os.path.isfile", return_value=True):
|
||||
with patch("os.access", return_value=True):
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_is_rust_analyzer_functional", return_value=True):
|
||||
result = RustAnalyzer.DependencyProvider._ensure_rust_analyzer_installed()
|
||||
|
||||
assert result == "/custom/bin/rust-analyzer"
|
||||
mock_which.assert_called_with("rust-analyzer")
|
||||
|
||||
@pytest.mark.rust
|
||||
def test_detect_from_nix_path_without_rustup(self):
|
||||
"""
|
||||
GIVEN rustup is NOT installed
|
||||
AND rust-analyzer is installed via Nix and available in PATH
|
||||
WHEN _ensure_rust_analyzer_installed is called
|
||||
THEN it should find and use the Nix-provided rust-analyzer
|
||||
|
||||
WHY: NixOS users install rust-analyzer via home-manager or nix-env,
|
||||
which places it in ~/.nix-profile/bin/ on PATH. Without rustup,
|
||||
Serena should still detect and use this binary.
|
||||
Fixes: https://github.com/oraios/serena/issues/800
|
||||
"""
|
||||
from solidlsp.language_servers.rust_analyzer import RustAnalyzer
|
||||
|
||||
nix_path = "/home/user/.nix-profile/bin/rust-analyzer"
|
||||
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rust_analyzer_via_rustup", return_value=None):
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rustup_version", return_value=None):
|
||||
with patch("shutil.which", return_value=nix_path):
|
||||
with patch("os.path.isfile", return_value=True):
|
||||
with patch("os.access", return_value=True):
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_is_rust_analyzer_functional", return_value=True):
|
||||
result = RustAnalyzer.DependencyProvider._ensure_rust_analyzer_installed()
|
||||
|
||||
assert result == nix_path
|
||||
|
||||
@pytest.mark.rust
|
||||
def test_skip_broken_rustup_proxy_in_path(self):
|
||||
"""
|
||||
GIVEN rustup is NOT available via 'rustup which'
|
||||
AND rust-analyzer in PATH is a broken rustup proxy symlink
|
||||
WHEN _ensure_rust_analyzer_installed is called
|
||||
THEN it should skip the broken proxy and fall through to common paths
|
||||
|
||||
WHY: On some systems, rust-analyzer in PATH is a symlink to rustup.
|
||||
If rustup doesn't have the component installed, running the binary fails.
|
||||
We verify functionality with --version before accepting a PATH result.
|
||||
"""
|
||||
from solidlsp.language_servers.rust_analyzer import RustAnalyzer
|
||||
|
||||
# Use platform-appropriate paths and binary names
|
||||
home = pathlib.Path.home()
|
||||
if IS_WINDOWS:
|
||||
binary_name = "rust-analyzer.exe"
|
||||
broken_proxy_path = str(home / "AppData" / "Local" / "Microsoft" / "WindowsApps" / binary_name)
|
||||
cargo_path = str(home / ".cargo" / "bin" / binary_name)
|
||||
else:
|
||||
binary_name = "rust-analyzer"
|
||||
broken_proxy_path = "/usr/bin/rust-analyzer"
|
||||
cargo_path = os.path.expanduser("~/.cargo/bin/rust-analyzer")
|
||||
|
||||
def mock_isfile(path):
|
||||
return path in [broken_proxy_path, cargo_path]
|
||||
|
||||
def mock_access(path, mode):
|
||||
return path in [broken_proxy_path, cargo_path]
|
||||
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rust_analyzer_via_rustup", return_value=None):
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_get_rustup_version", return_value=None):
|
||||
with patch("shutil.which", return_value=broken_proxy_path):
|
||||
with patch("os.path.isfile", side_effect=mock_isfile):
|
||||
with patch("os.access", side_effect=mock_access):
|
||||
# The PATH binary is a broken rustup proxy
|
||||
with patch.object(RustAnalyzer.DependencyProvider, "_is_rust_analyzer_functional", return_value=False):
|
||||
result = RustAnalyzer.DependencyProvider._ensure_rust_analyzer_installed()
|
||||
|
||||
# Should skip broken proxy and find cargo-installed version
|
||||
assert result == cargo_path
|
||||
|
||||
@pytest.mark.rust
|
||||
@pytest.mark.skipif(IS_WINDOWS, reason="Homebrew paths only apply to macOS/Linux")
|
||||
def test_detect_from_homebrew_apple_silicon_path(self):
|
||||
|
||||
Reference in new issue
Block a user