Skip to content

Commit 707703c

Browse files
committed
refactor: deduplicate Python language support code
Extract shared helpers and remove dead code across the language support area: - Extract `is_assignment_used()` and move `recurse_sections` to unused_definition_remover.py, replacing duplicated logic in both context files - Extract `function_sources_to_helpers()` in support.py to unify identical HelperFunction construction - Remove dead `get_comment_prefix()` method from protocol and all implementations (comment_prefix property serves all callers)
1 parent 82b4002 commit 707703c

5 files changed

Lines changed: 106 additions & 176 deletions

File tree

codeflash/languages/base.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -519,15 +519,6 @@ def get_test_file_suffix(self) -> str:
519519
"""
520520
...
521521

522-
def get_comment_prefix(self) -> str:
523-
"""Get the comment prefix for this language.
524-
525-
Returns:
526-
Comment prefix (e.g., "//" for JS, "#" for Python).
527-
528-
"""
529-
...
530-
531522
def find_test_root(self, project_root: Path) -> Path | None:
532523
"""Find the test root directory for a project.
533524

codeflash/languages/javascript/support.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,15 +1805,6 @@ def get_test_file_suffix(self) -> str:
18051805
"""
18061806
return ".test.js"
18071807

1808-
def get_comment_prefix(self) -> str:
1809-
"""Get the comment prefix for JavaScript.
1810-
1811-
Returns:
1812-
JavaScript single-line comment prefix.
1813-
1814-
"""
1815-
return "//"
1816-
18171808
def find_test_root(self, project_root: Path) -> Path | None:
18181809
"""Find the test root directory for a JavaScript project.
18191810

codeflash/languages/python/context/code_context_extractor.py

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
from codeflash.languages import Language, is_python
2121
from codeflash.languages.python.context.unused_definition_remover import (
2222
collect_top_level_defs_with_usages,
23-
extract_names_from_targets,
2423
get_section_names,
24+
is_assignment_used,
25+
recurse_sections,
2526
remove_unused_definitions_by_function_names,
2627
)
2728
from codeflash.models.models import (
@@ -34,8 +35,6 @@
3435
from codeflash.optimization.function_context import belongs_to_function_qualified
3536

3637
if TYPE_CHECKING:
37-
from collections.abc import Callable
38-
3938
from jedi.api.classes import Name
4039

4140
from codeflash.languages.base import HelperFunction
@@ -1103,50 +1102,6 @@ def _validate_classdef(node: cst.ClassDef, prefix: str) -> tuple[str, cst.Indent
11031102
return _qualified_name(prefix, node.name.value), node.body
11041103

11051104

1106-
def _recurse_sections(
1107-
node: cst.CSTNode,
1108-
section_names: list[str],
1109-
prune_fn: Callable[[cst.CSTNode], tuple[cst.CSTNode | None, bool]],
1110-
keep_non_target_children: bool = False,
1111-
) -> tuple[cst.CSTNode | None, bool]:
1112-
updates: dict[str, list[cst.CSTNode] | cst.CSTNode] = {}
1113-
found_any_target = False
1114-
for section in section_names:
1115-
original_content = getattr(node, section, None)
1116-
if isinstance(original_content, (list, tuple)):
1117-
new_children = []
1118-
section_found_target = False
1119-
for child in original_content:
1120-
filtered, found_target = prune_fn(child)
1121-
if filtered:
1122-
new_children.append(filtered)
1123-
section_found_target |= found_target
1124-
if keep_non_target_children:
1125-
if section_found_target or new_children:
1126-
found_any_target |= section_found_target
1127-
updates[section] = new_children
1128-
elif section_found_target:
1129-
found_any_target = True
1130-
updates[section] = new_children
1131-
elif original_content is not None:
1132-
filtered, found_target = prune_fn(original_content)
1133-
if keep_non_target_children:
1134-
found_any_target |= found_target
1135-
if filtered:
1136-
updates[section] = filtered
1137-
elif found_target:
1138-
found_any_target = True
1139-
if filtered:
1140-
updates[section] = filtered
1141-
if keep_non_target_children:
1142-
if updates:
1143-
return node.with_changes(**updates), found_any_target
1144-
return None, False
1145-
if not found_any_target:
1146-
return None, False
1147-
return (node.with_changes(**updates) if updates else node), True
1148-
1149-
11501105
def prune_cst(
11511106
node: cst.CSTNode,
11521107
target_functions: set[str],
@@ -1278,19 +1233,9 @@ def prune_cst(
12781233

12791234
# Handle assignments for READ_WRITABLE mode
12801235
if defs_with_usages is not None:
1281-
if isinstance(node, cst.Assign):
1282-
for target in node.targets:
1283-
names = extract_names_from_targets(target.target)
1284-
for name in names:
1285-
if name in defs_with_usages and defs_with_usages[name].used_by_qualified_function:
1286-
return node, True
1287-
return None, False
1288-
1289-
if isinstance(node, (cst.AnnAssign, cst.AugAssign)):
1290-
names = extract_names_from_targets(node.target)
1291-
for name in names:
1292-
if name in defs_with_usages and defs_with_usages[name].used_by_qualified_function:
1293-
return node, True
1236+
if isinstance(node, (cst.Assign, cst.AnnAssign, cst.AugAssign)):
1237+
if is_assignment_used(node, defs_with_usages):
1238+
return node, True
12941239
return None, False
12951240

12961241
# For other nodes, recursively process children
@@ -1299,7 +1244,7 @@ def prune_cst(
12991244
return node, False
13001245

13011246
if helpers is not None:
1302-
return _recurse_sections(
1247+
return recurse_sections(
13031248
node,
13041249
section_names,
13051250
lambda child: prune_cst(
@@ -1317,7 +1262,7 @@ def prune_cst(
13171262
),
13181263
keep_non_target_children=True,
13191264
)
1320-
return _recurse_sections(
1265+
return recurse_sections(
13211266
node,
13221267
section_names,
13231268
lambda child: prune_cst(

codeflash/languages/python/context/unused_definition_remover.py

Lines changed: 81 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
from codeflash.models.models import CodeString, CodeStringsMarkdown
1616

1717
if TYPE_CHECKING:
18+
from collections.abc import Callable
19+
1820
from codeflash.discovery.functions_to_optimize import FunctionToOptimize
1921
from codeflash.models.models import CodeOptimizationContext, FunctionSource
2022

@@ -49,6 +51,73 @@ def extract_names_from_targets(target: cst.CSTNode) -> list[str]:
4951
return names
5052

5153

54+
def is_assignment_used(
55+
node: cst.CSTNode,
56+
definitions: dict[str, UsageInfo],
57+
name_prefix: str = "",
58+
) -> bool:
59+
if isinstance(node, cst.Assign):
60+
for target in node.targets:
61+
names = extract_names_from_targets(target.target)
62+
for name in names:
63+
lookup = f"{name_prefix}{name}" if name_prefix else name
64+
if lookup in definitions and definitions[lookup].used_by_qualified_function:
65+
return True
66+
return False
67+
if isinstance(node, (cst.AnnAssign, cst.AugAssign)):
68+
names = extract_names_from_targets(node.target)
69+
for name in names:
70+
lookup = f"{name_prefix}{name}" if name_prefix else name
71+
if lookup in definitions and definitions[lookup].used_by_qualified_function:
72+
return True
73+
return False
74+
return False
75+
76+
77+
def recurse_sections(
78+
node: cst.CSTNode,
79+
section_names: list[str],
80+
prune_fn: Callable[[cst.CSTNode], tuple[cst.CSTNode | None, bool]],
81+
keep_non_target_children: bool = False,
82+
) -> tuple[cst.CSTNode | None, bool]:
83+
updates: dict[str, list[cst.CSTNode] | cst.CSTNode] = {}
84+
found_any_target = False
85+
for section in section_names:
86+
original_content = getattr(node, section, None)
87+
if isinstance(original_content, (list, tuple)):
88+
new_children = []
89+
section_found_target = False
90+
for child in original_content:
91+
filtered, found_target = prune_fn(child)
92+
if filtered:
93+
new_children.append(filtered)
94+
section_found_target |= found_target
95+
if keep_non_target_children:
96+
if section_found_target or new_children:
97+
found_any_target |= section_found_target
98+
updates[section] = new_children
99+
elif section_found_target:
100+
found_any_target = True
101+
updates[section] = new_children
102+
elif original_content is not None:
103+
filtered, found_target = prune_fn(original_content)
104+
if keep_non_target_children:
105+
found_any_target |= found_target
106+
if filtered:
107+
updates[section] = filtered
108+
elif found_target:
109+
found_any_target = True
110+
if filtered:
111+
updates[section] = filtered
112+
if keep_non_target_children:
113+
if updates:
114+
return node.with_changes(**updates), found_any_target
115+
return None, False
116+
if not found_any_target:
117+
return None, False
118+
return (node.with_changes(**updates) if updates else node), True
119+
120+
52121
def collect_top_level_definitions(
53122
node: cst.CSTNode, definitions: Optional[dict[str, UsageInfo]] = None
54123
) -> dict[str, UsageInfo]:
@@ -423,27 +492,9 @@ def remove_unused_definitions_recursively(
423492
elif isinstance(statement, (cst.Assign, cst.AnnAssign, cst.AugAssign)):
424493
var_used = False
425494

426-
# Check if any variable in this assignment is used
427-
if isinstance(statement, cst.Assign):
428-
for target in statement.targets:
429-
names = extract_names_from_targets(target.target)
430-
for name in names:
431-
class_var_name = f"{class_name}.{name}"
432-
if (
433-
class_var_name in definitions
434-
and definitions[class_var_name].used_by_qualified_function
435-
):
436-
var_used = True
437-
method_or_var_used = True
438-
break
439-
elif isinstance(statement, (cst.AnnAssign, cst.AugAssign)):
440-
names = extract_names_from_targets(statement.target)
441-
for name in names:
442-
class_var_name = f"{class_name}.{name}"
443-
if class_var_name in definitions and definitions[class_var_name].used_by_qualified_function:
444-
var_used = True
445-
method_or_var_used = True
446-
break
495+
if is_assignment_used(statement, definitions, name_prefix=f"{class_name}."):
496+
var_used = True
497+
method_or_var_used = True
447498

448499
if var_used or class_has_dependencies:
449500
new_statements.append(statement)
@@ -459,56 +510,21 @@ def remove_unused_definitions_recursively(
459510

460511
return node, method_or_var_used or class_has_dependencies
461512

462-
# Handle assignments (Assign and AnnAssign)
463-
if isinstance(node, cst.Assign):
464-
for target in node.targets:
465-
names = extract_names_from_targets(target.target)
466-
for name in names:
467-
if name in definitions and definitions[name].used_by_qualified_function:
468-
return node, True
469-
return None, False
470-
471-
if isinstance(node, (cst.AnnAssign, cst.AugAssign)):
472-
names = extract_names_from_targets(node.target)
473-
for name in names:
474-
if name in definitions and definitions[name].used_by_qualified_function:
475-
return node, True
513+
# Handle assignments (Assign, AnnAssign, AugAssign)
514+
if isinstance(node, (cst.Assign, cst.AnnAssign, cst.AugAssign)):
515+
if is_assignment_used(node, definitions):
516+
return node, True
476517
return None, False
477518

478519
# For other nodes, recursively process children
479520
section_names = get_section_names(node)
480521
if not section_names:
481522
return node, False
482-
483-
updates = {}
484-
found_used = False
485-
486-
for section in section_names:
487-
original_content = getattr(node, section, None)
488-
if isinstance(original_content, (list, tuple)):
489-
new_children = []
490-
section_found_used = False
491-
492-
for child in original_content:
493-
filtered, used = remove_unused_definitions_recursively(child, definitions)
494-
if filtered:
495-
new_children.append(filtered)
496-
section_found_used |= used
497-
498-
if new_children or section_found_used:
499-
found_used |= section_found_used
500-
updates[section] = new_children
501-
elif original_content is not None:
502-
filtered, used = remove_unused_definitions_recursively(original_content, definitions)
503-
found_used |= used
504-
if filtered:
505-
updates[section] = filtered
506-
if not found_used:
507-
return None, False
508-
if updates:
509-
return node.with_changes(**updates), found_used
510-
511-
return node, False
523+
return recurse_sections(
524+
node,
525+
section_names,
526+
lambda child: remove_unused_definitions_recursively(child, definitions),
527+
)
512528

513529

514530
def collect_top_level_defs_with_usages(

0 commit comments

Comments
 (0)