tiennm99/serena
1
0 Fork 0
mirror of https://github.com/tiennm99/serena.git synced 2026-10-11 03:13:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1383 from oraios/bugfix/insert_after_constant

Browse Source 
Forbid using insert_after_symbol on symbols with no proper body
This commit is contained in:
Michael Panchenko authored and GitHub committed 2026-04-21 12:43:57 +02:00
2 parents 14cd3ef559 2c65578536
commit b2cce67206
8 files changed
+98 -265
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

CHANGELOG.md
+1
View File
@@ -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)
scripts/demo_run_tools.py
+1 -3
View File
@@ -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...")
src/serena/code_editor.py
+7
View File
@@ -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"):
src/serena/symbol.py
+31
View File
@@ -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
src/serena/tools/symbol_tools.py
+2 -2
View File
@@ -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
src/solidlsp/ls_utils.py
+9
View File
@@ -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:
"""
test/serena/__snapshots__/test_symbol_editing.ambr
+47 -251
View File
@@ -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;
test/serena/test_symbol_editing.py
-9
View File
@@ -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,