From d39d16852359389dccaed24d8dbf703d57089e6c Mon Sep 17 00:00:00 2001 From: VoidChecksum <89574102+VoidChecksum@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:01:31 +0100 Subject: [PATCH] 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 --- .../language_servers/rust_analyzer.py | 35 +++++-- .../rust/test_rust_analyzer_detection.py | 98 ++++++++++++++++--- 2 files changed, 107 insertions(+), 26 deletions(-) diff --git a/src/solidlsp/language_servers/rust_analyzer.py b/src/solidlsp/language_servers/rust_analyzer.py index b7c95947..f0ff00e6 100644 --- a/src/solidlsp/language_servers/rust_analyzer.py +++ b/src/solidlsp/language_servers/rust_analyzer.py @@ -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 = [ diff --git a/test/solidlsp/rust/test_rust_analyzer_detection.py b/test/solidlsp/rust/test_rust_analyzer_detection.py index 85a978c3..fea2bb0e 100644 --- a/test/solidlsp/rust/test_rust_analyzer_detection.py +++ b/test/solidlsp/rust/test_rust_analyzer_detection.py @@ -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):