mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 14:16:21 +00:00
fix: Extend Nix symbol ranges to avoid additional semicolon
Add _extend_nix_symbol_range_to_include_semicolon method to prevent double semicolons Add test_nix_symbol_replacement_no_double_semicolon to validate the fix
This commit is contained in:
1 parent
51b2b0cea8
commit
b0ae69e468
5 files changed
+1256
-958
No files matched your search
@@ -15,6 +15,7 @@ from pathlib import Path
|
||||
|
||||
from overrides import override
|
||||
|
||||
from solidlsp import ls_types
|
||||
from solidlsp.ls import SolidLanguageServer
|
||||
from solidlsp.ls_config import LanguageServerConfig
|
||||
from solidlsp.ls_logger import LanguageServerLogger
|
||||
@@ -28,6 +29,79 @@ class NixLanguageServer(SolidLanguageServer):
|
||||
Provides Nix specific instantiation of the LanguageServer class using nixd.
|
||||
"""
|
||||
|
||||
def _extend_nix_symbol_range_to_include_semicolon(
|
||||
self, symbol: ls_types.UnifiedSymbolInformation, file_content: str
|
||||
) -> ls_types.UnifiedSymbolInformation:
|
||||
"""
|
||||
Extend symbol range to include trailing semicolon for Nix attribute symbols.
|
||||
|
||||
nixd provides ranges that exclude semicolons (expression-level), but serena needs
|
||||
statement-level ranges that include semicolons for proper replacement.
|
||||
"""
|
||||
range_info = symbol["range"]
|
||||
end_line = range_info["end"]["line"]
|
||||
end_char = range_info["end"]["character"]
|
||||
|
||||
# Split file content into lines
|
||||
lines = file_content.split("\n")
|
||||
if end_line >= len(lines):
|
||||
return symbol
|
||||
|
||||
line = lines[end_line]
|
||||
|
||||
# Check if there's a semicolon immediately after the current range end
|
||||
if end_char < len(line) and line[end_char] == ";":
|
||||
# Extend range to include the semicolon
|
||||
new_range = {"start": range_info["start"], "end": {"line": end_line, "character": end_char + 1}}
|
||||
|
||||
# Create modified symbol with extended range
|
||||
extended_symbol = symbol.copy()
|
||||
extended_symbol["range"] = new_range
|
||||
|
||||
# CRITICAL: Also update the location.range if it exists
|
||||
if extended_symbol.get("location"):
|
||||
location = extended_symbol["location"].copy()
|
||||
if "range" in location:
|
||||
location["range"] = new_range.copy()
|
||||
extended_symbol["location"] = location
|
||||
|
||||
return extended_symbol
|
||||
|
||||
return symbol
|
||||
|
||||
@override
|
||||
def request_document_symbols(
|
||||
self, relative_file_path: str, include_body: bool = False
|
||||
) -> tuple[list[ls_types.UnifiedSymbolInformation], list[ls_types.UnifiedSymbolInformation]]:
|
||||
"""
|
||||
Override to extend Nix symbol ranges to include trailing semicolons.
|
||||
|
||||
nixd provides expression-level ranges (excluding semicolons) but serena needs
|
||||
statement-level ranges (including semicolons) for proper symbol replacement.
|
||||
"""
|
||||
# Get symbols from parent implementation
|
||||
all_symbols, root_symbols = super().request_document_symbols(relative_file_path, include_body)
|
||||
|
||||
# Get file content for range extension
|
||||
file_content = self.language_server.retrieve_full_file_content(relative_file_path)
|
||||
|
||||
# Extend ranges for all symbols recursively
|
||||
def extend_symbol_and_children(symbol: ls_types.UnifiedSymbolInformation) -> ls_types.UnifiedSymbolInformation:
|
||||
# Extend this symbol's range
|
||||
extended = self._extend_nix_symbol_range_to_include_semicolon(symbol, file_content)
|
||||
|
||||
# Extend children recursively
|
||||
if extended.get("children"):
|
||||
extended["children"] = [extend_symbol_and_children(child) for child in extended["children"]]
|
||||
|
||||
return extended
|
||||
|
||||
# Apply range extension to all symbols
|
||||
extended_all_symbols = [extend_symbol_and_children(sym) for sym in all_symbols]
|
||||
extended_root_symbols = [extend_symbol_and_children(sym) for sym in root_symbols]
|
||||
|
||||
return extended_all_symbols, extended_root_symbols
|
||||
|
||||
@override
|
||||
def is_ignored_dirname(self, dirname: str) -> bool:
|
||||
# For Nix projects, we should ignore:
|
||||
|
||||
@@ -1,56 +1,56 @@
|
||||
# default.nix - Traditional Nix expression for backwards compatibility
|
||||
{ pkgs ? import <nixpkgs> {} }:
|
||||
{ pkgs ? import <nixpkgs> { } }:
|
||||
|
||||
let
|
||||
# Import library functions
|
||||
lib = pkgs.lib;
|
||||
stdenv = pkgs.stdenv;
|
||||
|
||||
|
||||
# Import our custom utilities
|
||||
utils = import ./lib/utils.nix { inherit lib; };
|
||||
|
||||
|
||||
# Custom function to create a greeting
|
||||
makeGreeting = name: "Hello, ${name}!";
|
||||
|
||||
|
||||
# List manipulation functions (using imported utils)
|
||||
listUtils = {
|
||||
double = list: map (x: x * 2) list;
|
||||
sum = list: lib.foldl' (acc: x: acc + x) 0 list;
|
||||
average = list:
|
||||
if list == []
|
||||
then 0
|
||||
average = list:
|
||||
if list == [ ]
|
||||
then 0
|
||||
else (listUtils.sum list) / (builtins.length list);
|
||||
# Use function from imported utils
|
||||
unique = utils.lists.unique;
|
||||
};
|
||||
|
||||
|
||||
# String utilities
|
||||
stringUtils = rec {
|
||||
capitalize = str:
|
||||
capitalize = str:
|
||||
let
|
||||
first = lib.substring 0 1 str;
|
||||
rest = lib.substring 1 (-1) str;
|
||||
in
|
||||
(lib.toUpper first) + rest;
|
||||
|
||||
(lib.toUpper first) + rest;
|
||||
|
||||
repeat = n: str: lib.concatStrings (lib.genList (_: str) n);
|
||||
|
||||
|
||||
padLeft = width: char: str:
|
||||
let
|
||||
len = lib.stringLength str;
|
||||
padding = if len >= width then 0 else width - len;
|
||||
in
|
||||
(repeat padding char) + str;
|
||||
(repeat padding char) + str;
|
||||
};
|
||||
|
||||
|
||||
# Package builder helper
|
||||
buildSimplePackage = { name, version, script }:
|
||||
stdenv.mkDerivation {
|
||||
pname = name;
|
||||
inherit version;
|
||||
|
||||
|
||||
phases = [ "installPhase" ];
|
||||
|
||||
|
||||
installPhase = ''
|
||||
mkdir -p $out/bin
|
||||
cat > $out/bin/${name} << EOF
|
||||
@@ -61,13 +61,14 @@ let
|
||||
'';
|
||||
};
|
||||
|
||||
in rec {
|
||||
in
|
||||
rec {
|
||||
# Export utilities
|
||||
inherit listUtils stringUtils makeGreeting;
|
||||
|
||||
|
||||
# Export imported utilities directly
|
||||
inherit (utils) math strings;
|
||||
|
||||
|
||||
# Example packages
|
||||
hello = buildSimplePackage {
|
||||
name = "hello";
|
||||
@@ -76,7 +77,7 @@ in rec {
|
||||
echo "${makeGreeting "World"}"
|
||||
'';
|
||||
};
|
||||
|
||||
|
||||
calculator = buildSimplePackage {
|
||||
name = "calculator";
|
||||
version = "0.1";
|
||||
@@ -95,7 +96,7 @@ in rec {
|
||||
esac
|
||||
'';
|
||||
};
|
||||
|
||||
|
||||
# Environment with multiple packages
|
||||
devEnv = pkgs.buildEnv {
|
||||
name = "dev-environment";
|
||||
@@ -107,7 +108,7 @@ in rec {
|
||||
calculator
|
||||
];
|
||||
};
|
||||
|
||||
|
||||
# Shell derivation
|
||||
shell = pkgs.mkShell {
|
||||
buildInputs = with pkgs; [
|
||||
@@ -117,20 +118,20 @@ in rec {
|
||||
gnugrep
|
||||
gnused
|
||||
];
|
||||
|
||||
|
||||
shellHook = ''
|
||||
echo "Entering Nix shell environment"
|
||||
echo "Available custom functions: makeGreeting, listUtils, stringUtils"
|
||||
'';
|
||||
};
|
||||
|
||||
|
||||
# Configuration example
|
||||
config = {
|
||||
system = {
|
||||
stateVersion = "23.11";
|
||||
enable = true;
|
||||
};
|
||||
|
||||
|
||||
services = {
|
||||
nginx = {
|
||||
enable = false;
|
||||
@@ -144,7 +145,7 @@ in rec {
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
users = {
|
||||
testUser = {
|
||||
name = "test";
|
||||
@@ -154,7 +155,7 @@ in rec {
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
# Recursive attribute set example
|
||||
tree = {
|
||||
root = {
|
||||
@@ -170,13 +171,13 @@ in rec {
|
||||
right = { value = 7; };
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
# Tree traversal function
|
||||
traverse = node:
|
||||
if node ? left && node ? right
|
||||
then [ node.value ] ++ (tree.traverse node.left) ++ (tree.traverse node.right)
|
||||
else if node ? value
|
||||
then [ node.value ]
|
||||
else [];
|
||||
else [ ];
|
||||
};
|
||||
}
|
||||
}
|
||||
File diff suppressed because it is too large.
Load diff
@@ -1,6 +1,7 @@
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
from abc import abstractmethod
|
||||
@@ -443,3 +444,42 @@ class ReplaceBodyTest(EditingTest):
|
||||
)
|
||||
def test_replace_body(test_case: ReplaceBodyTest, snapshot):
|
||||
test_case.run_test(content_after_ground_truth=snapshot)
|
||||
|
||||
|
||||
NIX_ATTR_REPLACEMENT = """c = 3;"""
|
||||
|
||||
|
||||
class NixAttrReplacementTest(EditingTest):
|
||||
"""Test for replacing individual attributes in Nix that should NOT result in double semicolons."""
|
||||
|
||||
def __init__(self, language: Language, rel_path: str, symbol_name: str, new_body: str):
|
||||
super().__init__(language, rel_path)
|
||||
self.symbol_name = symbol_name
|
||||
self.new_body = new_body
|
||||
|
||||
def _apply_edit(self, code_editor: CodeEditor) -> None:
|
||||
code_editor.replace_body(self.symbol_name, self.rel_path, self.new_body)
|
||||
|
||||
|
||||
@pytest.mark.nix
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="nixd language server doesn't run on Windows")
|
||||
def test_nix_symbol_replacement_no_double_semicolon(snapshot):
|
||||
"""
|
||||
Test that replacing a Nix attribute does not result in double semicolons.
|
||||
|
||||
This test exercises the bug where:
|
||||
- Original: users.users.example = { isSystemUser = true; group = "example"; description = "Example service user"; };
|
||||
- Replacement: c = 3;
|
||||
- Bug result would be: c = 3;; (double semicolon)
|
||||
- Correct result should be: c = 3; (single semicolon)
|
||||
|
||||
The replacement body includes a semicolon, but the language server's range extension
|
||||
logic should prevent double semicolons.
|
||||
"""
|
||||
test_case = NixAttrReplacementTest(
|
||||
Language.NIX,
|
||||
"default.nix",
|
||||
"testUser", # Simple attrset with multiple key-value pairs
|
||||
NIX_ATTR_REPLACEMENT,
|
||||
)
|
||||
test_case.run_test(content_after_ground_truth=snapshot)
|
||||
@@ -106,14 +106,14 @@ class TestNixLanguageServer:
|
||||
|
||||
assert refs is not None
|
||||
assert isinstance(refs, list)
|
||||
# nixd finds at least the inherit statement (line 66)
|
||||
# nixd finds at least the inherit statement (line 67)
|
||||
assert len(refs) >= 1, f"Should find at least 1 reference to makeGreeting, found {len(refs)}"
|
||||
|
||||
# Verify makeGreeting is referenced at expected locations
|
||||
if refs:
|
||||
ref_lines = sorted([ref["range"]["start"]["line"] for ref in refs])
|
||||
# Check if we found the inherit (line 66, 0-indexed: 65)
|
||||
assert 65 in ref_lines, f"Should find makeGreeting inherit at line 66, found at lines {[l+1 for l in ref_lines]}"
|
||||
# Check if we found the inherit (line 67, 0-indexed: 66)
|
||||
assert 66 in ref_lines, f"Should find makeGreeting inherit at line 67, found at lines {[l+1 for l in ref_lines]}"
|
||||
|
||||
@pytest.mark.parametrize("language_server", [Language.NIX], indirect=True)
|
||||
def test_hover_information(self, language_server: SolidLanguageServer) -> None:
|
||||
|
||||
Reference in new issue
Block a user