diff --git a/CHANGELOG.md b/CHANGELOG.md index 1be82cdc..81d731a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Status of the `main` branch. Changes prior to the next official version change w - Security: Forbid `".."` in memory names to disallow accessing files outside dedicated memory directories - Security: Add check for tool being read-only in the project server (previously only checked in `query_project` tool, i.e. client side) - Usage reporting now also includes the name of the Serena context that is used + - Fix: restricted `insert_after_symbol` to raise if used on an assignment or similar (can't reliably determine the symbol range) * JetBrains: - `Move` and `SafeDelete` tools: transform empty string to None (counteracts client errors) diff --git a/scripts/demo_run_tools.py b/scripts/demo_run_tools.py index d382dc32..b0ed063b 100644 --- a/scripts/demo_run_tools.py +++ b/scripts/demo_run_tools.py @@ -7,8 +7,6 @@ import json from pathlib import Path from pprint import pprint -from toon_format import encode - from serena.agent import SerenaAgent from serena.config.serena_config import SerenaConfig from serena.constants import REPO_ROOT @@ -46,5 +44,5 @@ if __name__ == "__main__": # keep_definition=True, ) ) - pprint(encode(json.loads(result))) + pprint(json.loads(result)) # input("Press Enter to continue...") diff --git a/src/serena/code_editor.py b/src/serena/code_editor.py index 2232f66b..2392d2ba 100644 --- a/src/serena/code_editor.py +++ b/src/serena/code_editor.py @@ -135,6 +135,13 @@ class CodeEditor(Generic[TSymbol], ABC): Inserts content after the symbol with the given name in the given file. """ symbol = self._find_unique_symbol(name_path, relative_file_path) + # Note: for body to be available, the symbol dto that the symbol instance is built from + # must have been retrieved either with body or at least with location. + # since _find_unique_symbol passes include_location=True, it works here + if symbol.body == symbol.name: + raise ValueError( + f"Cannot insert after this symbol (not a function, class or method): {symbol}. Consider using insert_before_symbol instead." + ) # make sure body always ends with at least one newline if not body.endswith("\n"): diff --git a/src/serena/symbol.py b/src/serena/symbol.py index 72aa4bbd..4a1b14cc 100644 --- a/src/serena/symbol.py +++ b/src/serena/symbol.py @@ -15,6 +15,7 @@ from solidlsp import SolidLanguageServer from solidlsp.ls import LSPFileBuffer from solidlsp.ls import ReferenceInSymbol as LSPReferenceInSymbol from solidlsp.ls_types import Position, SymbolKind, UnifiedSymbolInformation +from solidlsp.ls_utils import TextUtils from .ls_manager import LanguageServerManager from .project import Project @@ -89,6 +90,16 @@ class Symbol(ToStringMixin, ABC): def get_body_end_position(self) -> PositionInFile | None: pass + @property + @abstractmethod + def body(self) -> str | None: + pass + + @property + @abstractmethod + def name(self) -> str: + pass + def get_body_start_position_or_raise(self) -> PositionInFile: """ Get the start position of the symbol body, raising an error if it is not defined. @@ -879,6 +890,7 @@ class JetBrainsSymbol(Symbol): self._cached_file_content: str | None = None self._cached_body_start_position: PositionInFile | None = None self._cached_body_end_position: PositionInFile | None = None + self._cached_body = symbol_dict.get("body") def _tostring_includes(self) -> list[str]: return [] @@ -920,6 +932,25 @@ class JetBrainsSymbol(Symbol): self._cached_body_end_position = PositionInFile(line=line, col=col) return self._cached_body_end_position + @property + def body(self) -> str | None: + if self._cached_body is not None: + return self._cached_body + start_position = self.get_body_start_position() + if start_position is None: + return None + end_position = self.get_body_end_position() + assert end_position is not None, "If start position is available, end position should also be available. Symbol: {self}" + file_content = self.get_file_content() + self._cached_body = TextUtils.get_text_in_range( + file_content, start_position.line, start_position.col, end_position.line, end_position.col + ) + return self._cached_body + + @property + def name(self) -> str: + return self._dict["name_path"].split("/")[-1] + def is_neighbouring_definition_separated_by_empty_line(self) -> bool: # NOTE: Symbol types cannot really be differentiated, because types are not handled in a language-agnostic way. return False diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 40340e40..a19a99b9 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -369,8 +369,8 @@ class InsertAfterSymbolTool(Tool, ToolMarkerSymbolicEdit): body: str, ) -> str: """ - Inserts the given body/content after the end of the definition of the given symbol (via the symbol's location). - A typical use case is to insert a new class, function, method, field or variable assignment. + Use this to insert code after a class/method/function definition. + Don't use to insert after assignments (constants, fields). :param name_path: name path of the symbol after which to insert content (definitions in the `find_symbol` tool apply) :param relative_path: the relative path to the file containing the symbol diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 9db2a9a4..b54654b8 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -115,6 +115,15 @@ class TextUtils: new_l, new_c = TextUtils._get_updated_position_from_line_and_column_and_edit(line, col, text_to_be_inserted) return new_text, new_l, new_c + @staticmethod + def get_text_in_range(text: str, start_line: int, start_col: int, end_line: int, end_col: int) -> str: + """ + Returns the text between the given start and end positions. + """ + start_idx = TextUtils.get_index_from_line_col(text, start_line, start_col) + end_idx = TextUtils.get_index_from_line_col(text, end_line, end_col) + return text[start_idx:end_idx] + class PathUtils: """ diff --git a/test/serena/__snapshots__/test_symbol_editing.ambr b/test/serena/__snapshots__/test_symbol_editing.ambr index f614627e..6c688345 100644 --- a/test/serena/__snapshots__/test_symbol_editing.ambr +++ b/test/serena/__snapshots__/test_symbol_editing.ambr @@ -327,7 +327,6 @@ # Module-level variable with type annotation typed_module_var: int = 42 - new_module_var = "Inserted after typed_module_var" # Regular class with class and instance variables @@ -392,6 +391,9 @@ other_result = reassignable_module_var * 2 return result, other_result + def new_inserted_function(): + print("This is a new function inserted before another.") + # Create instances and use variables dataclass_instance = VariableDataclass(id=1, name="Test") @@ -428,7 +430,6 @@ reassignable_module_var = 20 # Reassigned # Module-level variable with type annotation - new_module_var = "Inserted after typed_module_var" typed_module_var: int = 42 @@ -488,6 +489,9 @@ # Function that uses the module variables + def new_inserted_function(): + print("This is a new function inserted before another.") + def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -511,209 +515,59 @@ # --- # name: test_insert_in_rel_to_symbol[test_case1-after] ''' - """ - Test module for variable declarations and usage. + export class DemoClass { + value: number; + constructor(value: number) { + this.value = value; + } + printValue() { + console.log(this.value); + } + } - This module tests various types of variable declarations and usages including: - - Module-level variables - - Class-level variables - - Instance variables - - Variable reassignments - """ + function newFunctionAfterClass(): void { + console.log("This function is after DemoClass."); + } - from dataclasses import dataclass, field + export function helperFunction() { + const demo = new DemoClass(42); + demo.printValue(); + } - # Module-level variables - module_var = "Initial module value" + helperFunction(); - reassignable_module_var = 10 - reassignable_module_var = 20 # Reassigned - - # Module-level variable with type annotation - typed_module_var: int = 42 - - - # Regular class with class and instance variables - class VariableContainer: - """Class that contains various variables.""" - - # Class-level variables - class_var = "Initial class value" - - reassignable_class_var = True - reassignable_class_var = False # Reassigned #noqa: PIE794 - - # Class-level variable with type annotation - typed_class_var: str = "typed value" - - def __init__(self): - # Instance variables - self.instance_var = "Initial instance value" - self.reassignable_instance_var = 100 - - # Instance variable with type annotation - self.typed_instance_var: list[str] = ["item1", "item2"] - - def modify_instance_var(self): - # Reassign instance variable - self.instance_var = "Modified instance value" - self.reassignable_instance_var = 200 # Reassigned - - def use_module_var(self): - # Use module-level variables - result = module_var + " used in method" - other_result = reassignable_module_var + 5 - return result, other_result - - def use_class_var(self): - # Use class-level variables - result = VariableContainer.class_var + " used in method" - other_result = VariableContainer.reassignable_class_var - return result, other_result - - - # Dataclass with variables - @dataclass - class VariableDataclass: - """Dataclass that contains various fields.""" - - # Field variables with type annotations - id: int - name: str - items: list[str] = field(default_factory=list) - metadata: dict[str, str] = field(default_factory=dict) - optional_value: float | None = None - - # This will be reassigned in various places - status: str = "pending" - - - # Function that uses the module variables - def use_module_variables(): - """Function that uses module-level variables.""" - result = module_var + " used in function" - other_result = reassignable_module_var * 2 - return result, other_result - - def new_inserted_function(): - print("This is a new function inserted before another.") - - - # Create instances and use variables - dataclass_instance = VariableDataclass(id=1, name="Test") - dataclass_instance.status = "active" # Reassign dataclass field - - # Use variables at module level - module_result = module_var + " used at module level" - other_module_result = reassignable_module_var + 30 - - # Create a second dataclass instance with different status - second_dataclass = VariableDataclass(id=2, name="Another Test") - second_dataclass.status = "completed" # Another reassignment of status + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } ''' # --- # name: test_insert_in_rel_to_symbol[test_case1-before] ''' - """ - Test module for variable declarations and usage. + function newFunctionAfterClass(): void { + console.log("This function is after DemoClass."); + } - This module tests various types of variable declarations and usages including: - - Module-level variables - - Class-level variables - - Instance variables - - Variable reassignments - """ + export class DemoClass { + value: number; + constructor(value: number) { + this.value = value; + } + printValue() { + console.log(this.value); + } + } - from dataclasses import dataclass, field + export function helperFunction() { + const demo = new DemoClass(42); + demo.printValue(); + } - # Module-level variables - module_var = "Initial module value" + helperFunction(); - reassignable_module_var = 10 - reassignable_module_var = 20 # Reassigned - - # Module-level variable with type annotation - typed_module_var: int = 42 - - - # Regular class with class and instance variables - class VariableContainer: - """Class that contains various variables.""" - - # Class-level variables - class_var = "Initial class value" - - reassignable_class_var = True - reassignable_class_var = False # Reassigned #noqa: PIE794 - - # Class-level variable with type annotation - typed_class_var: str = "typed value" - - def __init__(self): - # Instance variables - self.instance_var = "Initial instance value" - self.reassignable_instance_var = 100 - - # Instance variable with type annotation - self.typed_instance_var: list[str] = ["item1", "item2"] - - def modify_instance_var(self): - # Reassign instance variable - self.instance_var = "Modified instance value" - self.reassignable_instance_var = 200 # Reassigned - - def use_module_var(self): - # Use module-level variables - result = module_var + " used in method" - other_result = reassignable_module_var + 5 - return result, other_result - - def use_class_var(self): - # Use class-level variables - result = VariableContainer.class_var + " used in method" - other_result = VariableContainer.reassignable_class_var - return result, other_result - - - # Dataclass with variables - @dataclass - class VariableDataclass: - """Dataclass that contains various fields.""" - - # Field variables with type annotations - id: int - name: str - items: list[str] = field(default_factory=list) - metadata: dict[str, str] = field(default_factory=dict) - optional_value: float | None = None - - # This will be reassigned in various places - status: str = "pending" - - - # Function that uses the module variables - def new_inserted_function(): - print("This is a new function inserted before another.") - - def use_module_variables(): - """Function that uses module-level variables.""" - result = module_var + " used in function" - other_result = reassignable_module_var * 2 - return result, other_result - - - # Create instances and use variables - dataclass_instance = VariableDataclass(id=1, name="Test") - dataclass_instance.status = "active" # Reassign dataclass field - - # Use variables at module level - module_result = module_var + " used at module level" - other_module_result = reassignable_module_var + 30 - - # Create a second dataclass instance with different status - second_dataclass = VariableDataclass(id=2, name="Another Test") - second_dataclass.status = "completed" # Another reassignment of status + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } ''' # --- @@ -729,64 +583,6 @@ } } - function newFunctionAfterClass(): void { - console.log("This function is after DemoClass."); - } - - export function helperFunction() { - const demo = new DemoClass(42); - demo.printValue(); - } - - helperFunction(); - - export function unusedStandaloneFunction(): string { - return "This function is not referenced anywhere"; - } - - ''' -# --- -# name: test_insert_in_rel_to_symbol[test_case2-before] - ''' - function newFunctionAfterClass(): void { - console.log("This function is after DemoClass."); - } - - export class DemoClass { - value: number; - constructor(value: number) { - this.value = value; - } - printValue() { - console.log(this.value); - } - } - - export function helperFunction() { - const demo = new DemoClass(42); - demo.printValue(); - } - - helperFunction(); - - export function unusedStandaloneFunction(): string { - return "This function is not referenced anywhere"; - } - - ''' -# --- -# name: test_insert_in_rel_to_symbol[test_case3-after] - ''' - export class DemoClass { - value: number; - constructor(value: number) { - this.value = value; - } - printValue() { - console.log(this.value); - } - } - export function helperFunction() { const demo = new DemoClass(42); demo.printValue(); @@ -804,7 +600,7 @@ ''' # --- -# name: test_insert_in_rel_to_symbol[test_case3-before] +# name: test_insert_in_rel_to_symbol[test_case2-before] ''' export class DemoClass { value: number; diff --git a/test/serena/test_symbol_editing.py b/test/serena/test_symbol_editing.py index d6760982..b6f8abf2 100644 --- a/test/serena/test_symbol_editing.py +++ b/test/serena/test_symbol_editing.py @@ -328,15 +328,6 @@ class InsertInRelToSymbolTest(EditingTest): @pytest.mark.parametrize( "test_case", [ - pytest.param( - InsertInRelToSymbolTest( - Language.PYTHON, - PYTHON_TEST_REL_FILE_PATH, - "typed_module_var", - NEW_PYTHON_VARIABLE, - ), - marks=pytest.mark.python, - ), pytest.param( InsertInRelToSymbolTest( Language.PYTHON,