From c1b7af4d4a64923b664742f59a38994b4da59fdc Mon Sep 17 00:00:00 2001 From: tomcodegen Date: Fri, 14 Mar 2025 21:44:26 -0700 Subject: [PATCH 1/6] mv refactor --- .../sdk/codebase/transaction_manager.py | 17 + src/codegen/sdk/core/file.py | 8 + src/codegen/sdk/core/import_resolution.py | 25 +- src/codegen/sdk/core/symbol.py | 217 +- src/codegen/sdk/typescript/symbol.py | 188 +- .../file/test_file_remove_unused_import.py | 284 +++ .../sdk/python/file/test_file_unicode.py | 2 +- .../function/test_function_move_to_file.py | 324 ++- .../sdk/typescript/file/test_file_remove.py | 196 ++ .../sdk/typescript/file/test_file_unicode.py | 6 +- .../function/test_function_move_to_file.py | 48 +- .../move_symbol_to_file/test_move.py | 1750 +++++++++++++++++ .../test_move_tsx_to_file.py | 102 +- .../sdk/typescript/tsx/test_tsx_edit.py | 2 +- .../sdk/typescript/tsx/test_tsx_parsing.py | 2 +- .../guides/organize-your-codebase.py | 2 +- 16 files changed, 2896 insertions(+), 277 deletions(-) create mode 100644 tests/unit/codegen/sdk/python/file/test_file_remove_unused_import.py create mode 100644 tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move.py diff --git a/src/codegen/sdk/codebase/transaction_manager.py b/src/codegen/sdk/codebase/transaction_manager.py index a59b6eb4e..6aa0a6598 100644 --- a/src/codegen/sdk/codebase/transaction_manager.py +++ b/src/codegen/sdk/codebase/transaction_manager.py @@ -1,3 +1,4 @@ +import math import time from collections.abc import Callable from pathlib import Path @@ -289,6 +290,22 @@ def get_transactions_at_range(self, file_path: Path, start_byte: int, end_byte: return matching_transactions + def get_transaction_containing_range(self, file_path: Path, start_byte: int, end_byte: int, transaction_order: TransactionPriority | None = None) -> Transaction|None: + """Returns the nearest transaction that includes the range specified given the filtering criteria.""" + if file_path not in self.queued_transactions: + return None + + smallest_difference=math.inf + best_fit_transaction=None + for t in self.queued_transactions[file_path]: + if t.start_byte <= start_byte and t.end_byte>=end_byte: + if transaction_order is None or t.transaction_order == transaction_order: + smallest_difference = min(smallest_difference,abs(t.start_byte-start_byte)+abs(t.end_byte-end_byte)) + if smallest_difference==0: + return t + best_fit_transaction=t + return best_fit_transaction + def _get_conflicts(self, transaction: Transaction) -> list[Transaction]: """Returns all transactions that overlap with the given transaction""" overlapping_transactions = [] diff --git a/src/codegen/sdk/core/file.py b/src/codegen/sdk/core/file.py index 12bcab303..5da6ec980 100644 --- a/src/codegen/sdk/core/file.py +++ b/src/codegen/sdk/core/file.py @@ -943,6 +943,14 @@ def remove_unused_exports(self) -> None: None """ + + def remove_unused_imports(self) -> None: + # Process each import statement + for import_stmt in self.imports: + # Don't remove imports we can't be sure about + if import_stmt.usage_is_ascertainable(): + import_stmt.remove_if_unused() + #################################################################################################################### # MANIPULATIONS #################################################################################################################### diff --git a/src/codegen/sdk/core/import_resolution.py b/src/codegen/sdk/core/import_resolution.py index c6d11af9d..6fdd2adfc 100644 --- a/src/codegen/sdk/core/import_resolution.py +++ b/src/codegen/sdk/core/import_resolution.py @@ -221,6 +221,19 @@ def is_symbol_import(self) -> bool: """ return not self.is_module_import() + @reader + def usage_is_ascertainable(self) -> bool: + """Returns True if we can determine for sure whether the import is unused or not. + + Returns: + bool: True if the usage can be ascertained for the import, False otherwise. + """ + if self.is_wildcard_import() or self.is_sideffect_import(): + return False + return True + + + @reader def is_wildcard_import(self) -> bool: """Returns True if the import symbol is a wildcard import. @@ -234,6 +247,16 @@ def is_wildcard_import(self) -> bool: """ return self.import_type == ImportType.WILDCARD + @reader + def is_sideffect_import(self) -> bool: + #Maybe better name for this + """Determines if this is a sideffect. + + Returns: + bool: True if this is a sideffect import, False otherwise + """ + return self.import_type==ImportType.SIDE_EFFECT + @property @abstractmethod def namespace(self) -> str | None: @@ -663,7 +686,7 @@ def __eq__(self, other: object): @reader def remove_if_unused(self) -> None: if all( - self.transaction_manager.get_transactions_at_range(self.filepath, start_byte=usage.match.start_byte, end_byte=usage.match.end_byte, transaction_order=TransactionPriority.Remove) + self.transaction_manager.get_transaction_containing_range(self.file.path, start_byte=usage.match.start_byte, end_byte=usage.match.end_byte, transaction_order=TransactionPriority.Remove) for usage in self.usages ): self.remove() diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index cc0238b45..81bdbe6ee 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -5,6 +5,7 @@ from rich.markup import escape +from codegen.sdk.codebase.transactions import TransactionPriority from codegen.sdk.core.autocommit import commiter, reader, writer from codegen.sdk.core.dataclasses.usage import UsageKind, UsageType from codegen.sdk.core.detached_symbols.argument import Argument @@ -271,6 +272,7 @@ def move_to_file( file: SourceFile, include_dependencies: bool = True, strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", + cleanup_unused_imports: bool = True ) -> None: """Moves the given symbol to a new file and updates its imports and references. @@ -290,106 +292,167 @@ def move_to_file( AssertionError: If an invalid strategy is provided. """ encountered_symbols = {self} - self._move_to_file(file, encountered_symbols, include_dependencies, strategy) + self._move_to_file(file, encountered_symbols, include_dependencies, strategy, cleanup_unused_imports) @noapidoc + def _move_dependencies(self, + file: SourceFile, + encountered_symbols: set[Symbol | Import], + strategy:Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports" + ): + from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: + if dep in encountered_symbols: + continue + + # =====[ Symbols - move over ]===== + if isinstance(dep, Symbol) and dep.is_top_level: + encountered_symbols.add(dep) + dep._move_to_file( + file=file, + encountered_symbols=encountered_symbols, + include_dependencies=True, + strategy=strategy, + ) + + # =====[ Imports - copy over ]===== + elif isinstance(dep, Import): + if dep.imported_symbol: + file.add_import(imp=dep.imported_symbol, alias=dep.alias.source,import_type=dep.import_type) + else: + file.add_import(imp=dep.source) + + @noapidoc + def _update_dependencies_on_move(self,file:SourceFile): + from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: + # =====[ Symbols - add back edge ]===== + if isinstance(dep, Symbol) and dep.is_top_level: + file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=False) + elif isinstance(dep, Import): + if dep.imported_symbol: + file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) + else: + file.add_import(imp=dep.source) + + @noapidoc + def _execute_post_move_correction_strategy(self, + file:SourceFile, + encountered_symbols: set[Symbol | Import], + strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], + ): + from codegen.sdk.core.import_resolution import Import + import_line = self.get_import_string(module=file.import_module_name) + is_used_in_file = any( + usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols + for usage in self.symbol_usages + ) + match strategy: + case "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.remove() + case "add_back_edge": + if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.file.add_import(imp=import_line) + + # Delete the original symbol + self.remove() + case "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: + # Add updated import + usage.usage_symbol.file.add_import(import_line) + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(imp=import_line) + + # Add the import to the original file + if is_used_in_file: + self.file.add_import(imp=import_line) + # Delete the original symbol + self.remove() + @noapidoc def _move_to_file( self, file: SourceFile, encountered_symbols: set[Symbol | Import], include_dependencies: bool = True, strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", + cleanup_unused_imports: bool = True ) -> tuple[NodeId, NodeId]: """Helper recursive function for `move_to_file`""" - from codegen.sdk.core.import_resolution import Import - # =====[ Arg checking ]===== if file == self.file: return file.file_node_id, self.node_id + + #This removes any imports in the file we're moving the symbol to if imp := file.get_import(self.name): encountered_symbols.add(imp) imp.remove() + #This handles the new file + # =====[ Move over dependencies recursively ]===== if include_dependencies: - # =====[ Move over dependencies recursively ]===== - for dep in self.dependencies: - if dep in encountered_symbols: - continue - - # =====[ Symbols - move over ]===== - if isinstance(dep, Symbol) and dep.is_top_level: - encountered_symbols.add(dep) - dep._move_to_file( - file=file, - encountered_symbols=encountered_symbols, - include_dependencies=include_dependencies, - strategy=strategy, - ) - - # =====[ Imports - copy over ]===== - elif isinstance(dep, Import): - if dep.imported_symbol: - file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) - else: - file.add_import(imp=dep.source) + self._move_dependencies(file,encountered_symbols,strategy) else: - for dep in self.dependencies: - # =====[ Symbols - add back edge ]===== - if isinstance(dep, Symbol) and dep.is_top_level: - file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=False) - elif isinstance(dep, Import): - if dep.imported_symbol: - file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) - else: - file.add_import(imp=dep.source) + self._update_dependencies_on_move(file) # =====[ Make a new symbol in the new file ]===== - file.add_symbol(self) - import_line = self.get_import_string(module=file.import_module_name) - + should_export = False + if hasattr(self, "is_exported") and (self.is_exported or [ + usage for usage in self.usages + if usage.usage_symbol + not in encountered_symbols and not self.transaction_manager.get_transaction_containing_range( + usage.usage_symbol.file.path,usage.usage_symbol.start_byte,usage.usage_symbol.end_byte,TransactionPriority.Remove + ) + ]): + should_export=True + file.add_symbol(self,should_export=should_export) # =====[ Checks if symbol is used in original file ]===== # Takes into account that it's dependencies will be moved - is_used_in_file = any( - usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols and (usage.start_byte < self.start_byte or usage.end_byte > self.end_byte) # HACK - for usage in self.symbol_usages - ) - - # ======[ Strategy: Duplicate Dependencies ]===== - if strategy == "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - - # ======[ Strategy: Add Back Edge ]===== - # Here, we will add a "back edge" to the old file importing the symbol - elif strategy == "add_back_edge": - if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.file.add_import(imp=import_line) - # Delete the original symbol - self.remove() - - # ======[ Strategy: Update All Imports ]===== - # Update the imports in all the files which use this symbol to get it from the new file now - elif strategy == "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: - # Add updated import - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) - usage.usage_symbol.file.add_import(imp=import_line) - - # Add the import to the original file - if is_used_in_file: - self.file.add_import(imp=import_line) - # Delete the original symbol - self.remove() + from codegen.sdk.core.import_resolution import Import + + self._execute_post_move_correction_strategy(file,encountered_symbols,strategy) + + # =====[ Remove any imports that are no longer used ]===== + if cleanup_unused_imports: + for dep in self.dependencies: + if strategy!='duplicate_dependencies': + other_usages= [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] + else: + other_usages= [usage.usage_symbol for usage in dep.usages] + if isinstance(dep,Import): + dep.remove_if_unused() + + if not other_usages: + #Clean up forward... + dep.remove() + elif isinstance(dep,Symbol): + usages_in_file = [ + symb for symb in other_usages + if symb.file==self.file and not self.transaction_manager.get_transaction_containing_range( + symb.file.path,symb.start_byte,symb.end_byte,TransactionPriority.Remove + ) + ] + if self.transaction_manager.get_transaction_containing_range( + dep.file.path,dep.start_byte,dep.end_byte,transaction_order=TransactionPriority.Remove): + if not usages_in_file and strategy!="add_back_edge": + #We are going to assume there is only one such import + if imp_list :=[import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: + if insert_import_list := [ + transaction for transaction in self.transaction_manager.queued_transactions[self.file.path] + if imp_list[0] and transaction.new_content and imp_list[0] in transaction.new_content and transaction.transaction_order==TransactionPriority.Insert + ]: + self.transaction_manager.queued_transactions[self.file.path].remove(insert_import_list[0]) + self.file._pending_imports.remove(imp_list[0]) + @property @reader diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index e3cc89828..813d84490 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Literal, Self, Unpack +from typing import TYPE_CHECKING, Literal, Self, Unpack, override from codegen.sdk.core.assignment import Assignment from codegen.sdk.core.autocommit import reader, writer @@ -27,7 +27,6 @@ from codegen.sdk.core.file import SourceFile from codegen.sdk.core.import_resolution import Import from codegen.sdk.core.interfaces.editable import Editable - from codegen.sdk.core.node_id_factory import NodeId from codegen.sdk.typescript.detached_symbols.code_block import TSCodeBlock from codegen.sdk.typescript.interfaces.has_block import TSHasBlock @@ -255,117 +254,78 @@ def has_semicolon(self) -> bool: return self.semicolon_node is not None @noapidoc - def _move_to_file( - self, - file: SourceFile, - encountered_symbols: set[Symbol | Import], - include_dependencies: bool = True, - strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", - ) -> tuple[NodeId, NodeId]: - # TODO: Prevent creation of import loops (!) - raise a ValueError and make the agent fix it - # =====[ Arg checking ]===== - if file == self.file: - return file.file_node_id, self.node_id - - # =====[ Move over dependencies recursively ]===== - if include_dependencies: - try: - for dep in self.dependencies: - if dep in encountered_symbols: - continue - - # =====[ Symbols - move over ]===== - elif isinstance(dep, TSSymbol): - if dep.is_top_level: - encountered_symbols.add(dep) - dep._move_to_file(file, encountered_symbols=encountered_symbols, include_dependencies=True, strategy=strategy) - - # =====[ Imports - copy over ]===== - elif isinstance(dep, TSImport): - if dep.imported_symbol: - file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type) - else: - file.add_import(dep.source) - - else: - msg = f"Unknown dependency type {type(dep)}" - raise ValueError(msg) - except Exception as e: - print(f"Failed to move dependencies of {self.name}: {e}") - else: - try: - for dep in self.dependencies: - if isinstance(dep, Assignment): - msg = "Assignment not implemented yet" - raise NotImplementedError(msg) - - # =====[ Symbols - move over ]===== - elif isinstance(dep, Symbol) and dep.is_top_level: - file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=isinstance(dep, TypeAlias)) - - if not dep.is_exported: - dep.file.add_export_to_symbol(dep) - pass - - # =====[ Imports - copy over ]===== - elif isinstance(dep, TSImport): - if dep.imported_symbol: - file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type, is_type_import=dep.is_type_import()) - else: - file.add_import(dep.source) - - except Exception as e: - print(f"Failed to move dependencies of {self.name}: {e}") - - # =====[ Make a new symbol in the new file ]===== - # This will update all edges etc. - file.add_symbol(self) - import_line = self.get_import_string(module=file.import_module_name) - - # =====[ Checks if symbol is used in original file ]===== - # Takes into account that it's dependencies will be moved - is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) - - # ======[ Strategy: Duplicate Dependencies ]===== - if strategy == "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - - # ======[ Strategy: Add Back Edge ]===== - # Here, we will add a "back edge" to the old file importing the self - elif strategy == "add_back_edge": - if is_used_in_file: - self.file.add_import(import_line) - if self.is_exported: - self.file.add_import(f"export {{ {self.name} }}") - elif self.is_exported: - module_name = file.name - self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") - # Delete the original symbol - self.remove() - - # ======[ Strategy: Update All Imports ]===== - # Update the imports in all the files which use this symbol to get it from the new file now - elif strategy == "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, TSImport): - # Add updated import - if usage.usage_symbol.resolved_symbol is not None and usage.usage_symbol.resolved_symbol.node_type == NodeType.SYMBOL and usage.usage_symbol.resolved_symbol == self: - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) - usage.usage_symbol.file.add_import(import_line) - if is_used_in_file: - self.file.add_import(import_line) - # Delete the original symbol - self.remove() + @override + def _update_dependencies_on_move(self,file:SourceFile): + for dep in self.dependencies: + if isinstance(dep, Assignment): + msg = "Assignment not implemented yet" + raise NotImplementedError(msg) + + # =====[ Symbols - move over ]===== + elif isinstance(dep, TSSymbol) and dep.is_top_level: + file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=isinstance(dep, TypeAlias)) + + if not dep.is_exported: + dep.file.add_export_to_symbol(dep) + pass + # =====[ Imports - copy over ]===== + elif isinstance(dep, TSImport): + if dep.imported_symbol: + file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type, is_type_import=dep.is_type_import()) + else: + file.add_import(dep.source) + + @noapidoc + @override + def _execute_post_move_correction_strategy(self, + file:SourceFile, + encountered_symbols: set[Symbol | Import], + strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], + ): + import_line = self.get_import_string(module=file.import_module_name) + # =====[ Checks if symbol is used in original file ]===== + # Takes into account that it's dependencies will be moved + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) + + match strategy: + case "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL for usage in self.symbol_usages) + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.remove() + case "add_back_edge": + if is_used_in_file: + back_edge_line = import_line + if self.is_exported: + back_edge_line=back_edge_line.replace('import','export') + self.file.add_import(back_edge_line) + elif self.is_exported: + module_name = file.name + self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") + # Delete the original symbol + self.remove() + case "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, TSImport): + # Add updated import + if usage.usage_symbol.resolved_symbol is not None and usage.usage_symbol.resolved_symbol.node_type == NodeType.SYMBOL and usage.usage_symbol.resolved_symbol == self: + if usage.usage_symbol.file!=file: + # Just remove if the dep is now going to the file + usage.usage_symbol.file.add_import(import_line) + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(import_line) + + if is_used_in_file: + self.file.add_import(import_line) + # Delete the original symbol + self.remove() def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter | None, level: int) -> str: """Converts a PropType definition to its TypeScript equivalent.""" @@ -373,6 +333,8 @@ def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter type_map = {"string": "string", "number": "number", "bool": "boolean", "object": "object", "array": "any[]", "func": "CallableFunction"} if prop_type.source in type_map: return type_map[prop_type.source] + + if isinstance(prop_type, ChainedAttribute): if prop_type.attribute.source == "node": return "T" diff --git a/tests/unit/codegen/sdk/python/file/test_file_remove_unused_import.py b/tests/unit/codegen/sdk/python/file/test_file_remove_unused_import.py new file mode 100644 index 000000000..c36e8e52a --- /dev/null +++ b/tests/unit/codegen/sdk/python/file/test_file_remove_unused_import.py @@ -0,0 +1,284 @@ +from codegen.sdk.codebase.factory.get_session import get_codebase_session + + +def test_remove_unused_imports_basic(tmpdir) -> None: + """Test basic unused import removal""" + # language=python + content = """ +import os +import sys +from math import pi, sin +import json as jsonlib + +print(os.getcwd()) +sin(pi) +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "import sys" not in file.content + assert "import jsonlib" not in file.content + assert "import os" in file.content + assert "from math import pi, sin" in file.content + + +def test_remove_unused_imports_multiline(tmpdir) -> None: + """Test removal of unused imports in multiline import statements""" + # language=python + content = """ +from my_module import ( + used_func, + unused_func, + another_unused, + used_class, + unused_class +) + +result = used_func() +obj = used_class() +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "unused_func" not in file.content + assert "another_unused" not in file.content + assert "unused_class" not in file.content + assert "used_func" in file.content + assert "used_class" in file.content + + +def test_remove_unused_imports_with_aliases(tmpdir) -> None: + """Test removal of unused imports with aliases""" + # language=python + content = """ +from module import ( + long_name as short, + unused as alias, + used_thing as ut +) +import pandas as pd +import numpy as np + +print(short) +result = ut.process() +data = pd.DataFrame() +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "unused as alias" not in file.content + assert "numpy as np" not in file.content + assert "long_name as short" in file.content + assert "used_thing as ut" in file.content + assert "pandas as pd" in file.content + + +def test_remove_unused_imports_preserves_comments(tmpdir) -> None: + """Test that removing unused imports preserves relevant comments""" + # language=python + content = """ +# Important imports below +import os # Used for OS operations +import sys # Unused but commented +from math import ( # Math utilities + pi, # Circle constant + e, # Unused constant + sin # Trig function +) + +print(os.getcwd()) +print(sin(pi)) +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "# Important imports below" in file.content + assert "import os # Used for OS operations" in file.content + assert "import sys # Unused but commented" not in file.content + assert "e, # Unused constant" not in file.content + assert "pi, # Circle constant" in file.content + assert "sin # Trig function" in file.content + + +def test_remove_unused_imports_relative_imports(tmpdir) -> None: + """Test handling of relative imports""" + # language=python + content = """ +from . import used_module +from .. import unused_module +from .subpackage import used_thing, unused_thing +from ..utils import helper + +used_module.func() +used_thing.process() +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "from . import used_module" in file.content + assert "from .. import unused_module" not in file.content + assert "unused_thing" not in file.content + assert "from ..utils import helper" not in file.content + assert "used_thing" in file.content + + +def test_remove_unused_imports_star_imports(tmpdir) -> None: + """Test handling of star imports (should not be removed as we can't track usage)""" + # language=python + content = """ +from os import * +from sys import * +from math import pi +from math import sqrt + +getcwd() # from os +print(pi) +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "from os import *" in file.content + assert "from sys import *" in file.content + assert "from math import pi" in file.content + + +def test_remove_unused_imports_type_hints(tmpdir) -> None: + """Test handling of imports used in type hints""" + # language=python + content = """ +from typing import List, Dict, Optional, Any +from custom_types import CustomType, UnusedType + +def func(arg: List[int], opt: Optional[CustomType]) -> Dict[str, Any]: + return {"result": arg} +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert "List, Dict, Optional, Any" in file.content + assert "CustomType" in file.content + assert "UnusedType" not in file.content + + +def test_remove_unused_imports_empty_file(tmpdir) -> None: + """Test handling of empty files""" + # language=python + content = """ +# Empty file with imports +import os +import sys +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + file.remove_unused_imports() + + assert file.content.strip() == "# Empty file with imports" + + +def test_remove_unused_imports_multiple_removals(tmpdir) -> None: + """Test multiple rounds of import removal""" + # language=python + content = """ +import os +import sys +import json + +def func(): + print(os.getcwd()) +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + + # First removal + file.remove_unused_imports() + codebase.commit() + file = codebase.get_file("test.py") + + assert "import sys" not in file.content + assert "import json" not in file.content + assert "import os" in file.content + + # Second removal (should not change anything) + file.remove_unused_imports() + codebase.commit() + file = codebase.get_file("test.py") + + assert "import sys" not in file.content + assert "import json" not in file.content + assert "import os" in file.content + + +def test_file_complex_example_test_spliter(tmpdir) -> None: + """Test splitting a test file into multiple files, removing unused imports""" + # language=python + content = """ +from math import pi +from math import sqrt + +def test_set_comparison(): + set1 = set("1308") + set2 = set("8035") + assert set1 == set2 + +def test_math_sqrt(): + assert sqrt(4) == 2 +""" + with get_codebase_session(tmpdir=tmpdir, files={"test.py": content}) as codebase: + file = codebase.get_file("test.py") + base_name = "test_utils" + + # Group tests by subpath + test_groups = {} + for test_function in file.functions: + if test_function.name.startswith("test_"): + test_subpath = "_".join(test_function.name.split("_")[:3]) + if test_subpath not in test_groups: + test_groups[test_subpath] = [] + test_groups[test_subpath].append(test_function) + + # Print and process each group + for subpath, tests in test_groups.items(): + new_filename = f"{base_name}/{subpath}.py" + + # Create file if it doesn't exist + if not codebase.has_file(new_filename): + new_file = codebase.create_file(new_filename) + file = codebase.get_file(new_filename) + + # Move each test in the group + for test_function in tests: + print(f"Moving function {test_function.name} to {new_filename}") + test_function.move_to_file(new_file, strategy="update_all_imports", include_dependencies=True) + original_file = codebase.get_file("test.py") + + # Force a commit to ensure all changes are applied + codebase.commit() + + # Verify the results + # Check that original test.py is empty of test functions + original_file = codebase.get_file("test.py", optional=True) + assert original_file is not None + assert len([f for f in original_file.functions if f.name.startswith("test_")]) == 0 + + # Verify test_set_comparison was moved correctly + set_comparison_file = codebase.get_file("test_utils/test_set_comparison.py", optional=True) + assert set_comparison_file is not None + assert "test_set_comparison" in set_comparison_file.content + assert 'set1 = set("1308")' in set_comparison_file.content + + # Verify test_math_sqrt was moved correctly + math_file = codebase.get_file("test_utils/test_math_sqrt.py", optional=True) + assert math_file is not None + assert "test_math_sqrt" in math_file.content + assert "assert sqrt(4) == 2" in math_file.content + + # Verify imports were preserved + assert "from math import sqrt" in math_file.content + assert "from math import pi" not in math_file.content # Unused import should be removed diff --git a/tests/unit/codegen/sdk/python/file/test_file_unicode.py b/tests/unit/codegen/sdk/python/file/test_file_unicode.py index af1c0e73a..6509d5d77 100644 --- a/tests/unit/codegen/sdk/python/file/test_file_unicode.py +++ b/tests/unit/codegen/sdk/python/file/test_file_unicode.py @@ -39,7 +39,7 @@ def baz(): file3 = codebase.get_file("file3.py") bar = file2.get_function("bar") - bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge") + bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge",cleanup_unused_imports=False) assert file1.content == content1 # language=python diff --git a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py index 31dc17fa9..93059445f 100644 --- a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py +++ b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py @@ -46,8 +46,6 @@ def external_dep(): # language=python EXPECTED_FILE_2_CONTENT = """ -from file1 import external_dep - def foo(): return foo_dep() + 1 @@ -68,7 +66,6 @@ def bar(): return external_dep() + bar_dep() """ # =============================== - # TODO: [low] Should maybe remove unused external_dep? # TODO: [low] Missing newline after import with get_codebase_session( @@ -279,7 +276,7 @@ def baz(): assert isinstance(new_symbol, Function) -def test_move_to_file_add_back_edge(tmpdir) -> None: +def test_move_to_file_add_back_edge_internal_use(tmpdir) -> None: # ========== [ BEFORE ] ========== # language=python FILE_1_CONTENT = """ @@ -297,6 +294,9 @@ def foo(): def foo_dep(): return 24 +def use_bar(): + return 1 + bar() + def bar(): return external_dep() + bar_dep() @@ -321,8 +321,103 @@ def external_dep(): # language=python EXPECTED_FILE_2_CONTENT = """ +from file3 import bar +def foo(): + return foo_dep() + 1 + +def foo_dep(): + return 24 + +def use_bar(): + return 1 + bar() + +""" + + # language=python + EXPECTED_FILE_3_CONTENT = """ from file1 import external_dep +def baz(): + return bar() + 1 +def bar_dep(): + return 2 + +def bar(): + return external_dep() + bar_dep() +""" + + # =============================== + # TODO: [low] Missing newline after import + + with get_codebase_session( + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + "file3.py": FILE_3_CONTENT, + }, + ) as codebase: + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + file3 = codebase.get_file("file3.py") + + bar = file2.get_function("bar") + bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge") + + assert file1.content.strip() == EXPECTED_FILE_1_CONTENT.strip() + assert file2.content.strip() == EXPECTED_FILE_2_CONTENT.strip() + assert file3.content.strip() == EXPECTED_FILE_3_CONTENT.strip() + +def test_move_to_file_add_back_edge_external_use(tmpdir) -> None: + # ========== [ BEFORE ] ========== + # language=python + FILE_1_CONTENT = """ +def external_dep(): + return 42 +""" + + # language=python + FILE_2_CONTENT = """ +from file1 import external_dep + +def foo(): + return foo_dep() + 1 + +def foo_dep(): + return 24 + +def bar(): + return external_dep() + bar_dep() + +def bar_dep(): + return 2 +""" + + # language=python + FILE_3_CONTENT = """ +from file2 import bar + +def baz(): + return bar() + 1 +""" + FILE_4_CONTENT = """ +from file2 import bar + +def bla(): + return bar() + 1 +""" + + + # ========== [ AFTER ] ========== + # language=python + EXPECTED_FILE_1_CONTENT = """ +def external_dep(): + return 42 +""" + + # language=python + EXPECTED_FILE_2_CONTENT = """ +from file3 import bar def foo(): return foo_dep() + 1 @@ -343,8 +438,14 @@ def bar(): return external_dep() + bar_dep() """ + EXPECTED_FILE_4_CONTENT = """ +from file2 import bar + +def bla(): + return bar() + 1 + """ + # =============================== - # TODO: [low] Should maybe remove unused external_dep? # TODO: [low] Missing newline after import with get_codebase_session( @@ -353,11 +454,14 @@ def bar(): "file1.py": FILE_1_CONTENT, "file2.py": FILE_2_CONTENT, "file3.py": FILE_3_CONTENT, + "file4.py": FILE_4_CONTENT, + }, ) as codebase: file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") file3 = codebase.get_file("file3.py") + file4 = codebase.get_file("file4.py") bar = file2.get_function("bar") bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge") @@ -365,6 +469,7 @@ def bar(): assert file1.content.strip() == EXPECTED_FILE_1_CONTENT.strip() assert file2.content.strip() == EXPECTED_FILE_2_CONTENT.strip() assert file3.content.strip() == EXPECTED_FILE_3_CONTENT.strip() + assert file4.content.strip() == EXPECTED_FILE_4_CONTENT.strip() def test_move_to_file_add_back_edge_including_dependencies(tmpdir) -> None: @@ -601,8 +706,6 @@ def external_dep(): # language=python EXPECTED_FILE_2_CONTENT = """ -from file1 import external_dep - def foo(): return foo_dep() + 1 @@ -872,10 +975,7 @@ def test_move_global_var(tmpdir) -> None: """ # language=python - EXPECTED_FILE_2_CONTENT = """ -from import1 import thing1 -from import2 import thing2, thing3 -""" + EXPECTED_FILE_2_CONTENT = """""" # =============================== # TODO: [medium] Space messed up in file1 @@ -1311,8 +1411,6 @@ def bar(config: ExtendedConfig): # ========== [ AFTER ] ========== # language=python EXPECTED_FILE_1_CONTENT = """ -from dataclasses import dataclass - def foo(): return 1 """ @@ -1332,8 +1430,7 @@ class Config: # language=python EXPECTED_FILE_2_CONTENT = """ from file2.types import ExtendedConfig -from file1.types import Config -from dataclasses import dataclass + def bar(config: ExtendedConfig): '''Function that uses the dataclass''' @@ -1381,3 +1478,200 @@ class ExtendedConfig(Config): assert file1_types.content.strip() == EXPECTED_FILE_1_TYPES_CONTENT.strip() assert file2.content.strip() == EXPECTED_FILE_2_CONTENT.strip() assert file2_types.content.strip() == EXPECTED_FILE_2_TYPES_CONTENT.strip() + + + +def test_move_to_file_decorators(tmpdir) -> None: + # ========== [ BEFORE ] ========== + # language=python + FILE_1_CONTENT = """ + from test.foo import TEST + + test_decorator = TEST() + + @test_decorator.foo() + def test_func(): + pass +""" + + # language=python + FILE_2_CONTENT = """ + + """ + + with get_codebase_session( + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: + + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + + test_func = file1.get_function("test_func") + test_func.move_to_file(file2) + + codebase.commit() + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + + +def test_move_to_file_multiple_same_transaction(tmpdir) -> None: + # language=python + FILE_1_CONTENT = """ +from test.foo import TEST + +NO_MOVE=2 +def useful(): + pass + +def test_func(): + print(TEST) + +def foo(): + test_func() + useful() + +def bar(): + print(5) + useful() + +def boo(): + print(6) + useful() +""" + + # language=python + FILE_2_CONTENT = "NO_MOVE_FILE_2 = 6" + + FILE_1_EXPECTED =""" +NO_MOVE=2 +""" + FILE_2_EXPECTED =""" +from test.foo import TEST +NO_MOVE_FILE_2 = 6 + +def useful(): + pass + +def test_func(): + print(TEST) + +def foo(): + test_func() + useful() + +def bar(): + print(5) + useful() + +def boo(): + print(6) + useful() +""" + + with get_codebase_session( + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: + + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + + foo = file1.get_function("foo") + bar = file1.get_function("bar") + boo = file1.get_function("boo") + foo.move_to_file(file2) + bar.move_to_file(file2) + boo.move_to_file(file2) + + codebase.commit() + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + assert file1.source.strip() == FILE_1_EXPECTED.strip() + assert file2.source.strip() == FILE_2_EXPECTED.strip() + + + +def test_move_to_file_multiple_same_transaction_partial(tmpdir) -> None: + # language=python + FILE_1_CONTENT = """ +from test.foo import TEST + +NO_MOVE=2 +def useful(): + pass + +def test_func(): + print(TEST) + +def foo(): + test_func() + useful() + +def bar(): + print(5) + useful() + +def boo(): + print(6) + useful() +""" + + # language=python + FILE_2_CONTENT = "NO_MOVE_FILE_2 = 6" + + FILE_1_EXPECTED =""" +from file2 import useful +NO_MOVE=2 + +def boo(): + print(6) + useful() +""" + FILE_2_EXPECTED =""" +from test.foo import TEST +NO_MOVE_FILE_2 = 6 + +def useful(): + pass + +def test_func(): + print(TEST) + +def foo(): + test_func() + useful() + +def bar(): + print(5) + useful() +""" + + with get_codebase_session( + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: + + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + + foo = file1.get_function("foo") + bar = file1.get_function("bar") + boo = file1.get_function("boo") + foo.move_to_file(file2) + bar.move_to_file(file2) + + codebase.commit() + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + assert file1.source.strip() == FILE_1_EXPECTED.strip() + assert file2.source.strip() == FILE_2_EXPECTED.strip() diff --git a/tests/unit/codegen/sdk/typescript/file/test_file_remove.py b/tests/unit/codegen/sdk/typescript/file/test_file_remove.py index cca4fabcd..0fb07d96e 100644 --- a/tests/unit/codegen/sdk/typescript/file/test_file_remove.py +++ b/tests/unit/codegen/sdk/typescript/file/test_file_remove.py @@ -1,5 +1,7 @@ import os +import pytest + from codegen.sdk.codebase.factory.get_session import get_codebase_session from codegen.shared.enums.programming_language import ProgrammingLanguage @@ -16,3 +18,197 @@ def tets_remove_existing_file(tmpdir) -> None: file.remove() assert not os.path.exists(file.filepath) + + +def test_remove_unused_imports_complete_removal(tmpdir): + content = """ + import { unused1, unused2 } from './module1'; + import type { UnusedType } from './types'; + + const x = 5; + """ + expected = """ + const x = 5; + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"test.ts": content}) as codebase: + file = codebase.get_file("test.ts") + file.remove_unused_imports() + assert file.content.strip() == expected.strip() + + +def test_remove_unused_imports_partial_removal(tmpdir): + content = """ + import { used, unused } from './module1'; + + console.log(used); + """ + expected = """ + import { used } from './module1'; + + console.log(used); + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"test.ts": content}) as codebase: + file = codebase.get_file("test.ts") + file.remove_unused_imports() + assert file.content.strip() == expected.strip() + + +def test_remove_unused_imports_with_side_effects(tmpdir): + content = """ + import './styles.css'; + import { unused } from './module1'; + + const x = 5; + """ + expected = """ + import './styles.css'; + + const x = 5; + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"test.ts": content}) as codebase: + file = codebase.get_file("test.ts") + file.remove_unused_imports() + assert file.content.strip() == expected.strip() + + +def test_remove_unused_imports_with_moved_symbols(tmpdir): + content1 = """ + import { helper } from './utils'; + + export function foo() { + return helper(); + } + """ + # The original file should be empty after move since foo was the only content + expected1 = "" + + content2 = """ + export function helper() { + return true; + } + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"main.ts": content1, "utils.ts": content2}) as codebase: + main_file = codebase.get_file("main.ts") + foo = main_file.get_function("foo") + + # Move foo to a new file + new_file = codebase.create_file("new.ts") + foo.move_to_file(new_file,cleanup_unused_imports=False) + codebase.commit() + # Confirm cleanup false is respected + assert main_file.content.strip() == "import { helper } from './utils';" + + # Now explicitly remove unused imports after the move + main_file.remove_unused_imports() + assert main_file.content.strip() == "" + + +@pytest.mark.skip(reason="This test is not implemented properly yet") +def test_remove_unused_exports_with_side_effects(tmpdir): + content = """ +import './styles.css'; +export const unused = 5; +export function usedFunction() { return true; } + +const x = usedFunction(); + """ + expected = """ +import './styles.css'; +export function usedFunction() { return true; } + +const x = usedFunction(); + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"test.ts": content}) as codebase: + file = codebase.get_file("test.ts") + file.remove_unused_exports() + assert file.content.strip() == expected.strip() + + +@pytest.mark.skip(reason="This test is not implemented properly yet") +def test_remove_unused_exports_with_multiple_types(tmpdir): + content = """ +export const UNUSED_CONSTANT = 42; +export type UnusedType = string; +export interface UnusedInterface {} +export default function main() { return true; } +export function usedFunction() { return true; } +const x = usedFunction(); + """ + # Only value exports that are unused should be removed + expected = """ +export type UnusedType = string; +export interface UnusedInterface {} +export default function main() { return true; } +export function usedFunction() { return true; } +const x = usedFunction(); + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"test.ts": content}) as codebase: + file = codebase.get_file("test.ts") + file.remove_unused_exports() + assert file.content.strip() == expected.strip() + + +@pytest.mark.skip(reason="This test is not implemented properly yet") +def test_remove_unused_exports_with_reexports(tmpdir): + content1 = """ +export { helper } from './utils'; +export { unused } from './other'; +export function localFunction() { return true; } + """ + content2 = """ +import { helper } from './main'; +const x = helper(); + """ + expected1 = """ +export { helper } from './utils'; +export function localFunction() { return true; } + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"main.ts": content1, "other.ts": content2}) as codebase: + main_file = codebase.get_file("main.ts") + main_file.remove_unused_exports() + assert main_file.content.strip() == expected1.strip() + + +def test_remove_unused_exports_with_moved_and_reexported_symbol(tmpdir): + content1 = """ +export function helper() { + return true; +} + """ + content2 = """ +import { helper } from './utils'; +export { helper }; // This re-export should be preserved as it's used + +const x = helper(); + """ + content3 = """ +import { helper } from './main'; + +function useHelper() { + return helper(); +} + """ + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files={"utils.ts": content1, "main.ts": content2, "consumer.ts": content3}) as codebase: + utils_file = codebase.get_file("utils.ts") + main_file = codebase.get_file("main.ts") + consumer_file = codebase.get_file("consumer.ts") + # Move helper to main.ts + helper = utils_file.get_function("helper") + helper.move_to_file(main_file) + + # Remove unused exports + utils_file.remove_unused_exports() + main_file.remove_unused_exports() + + # The re-export in main.ts should be preserved since it's used by consumer.ts + assert "export { helper }" in main_file.content + # The original export in utils.ts should be gone + assert "export function helper" not in utils_file.content diff --git a/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py b/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py index 8beab6133..6ad356dad 100644 --- a/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py +++ b/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py @@ -47,7 +47,7 @@ def test_unicode_move_symbol(tmpdir) -> None: file3 = codebase.get_file("file3.ts") bar = file2.get_function("bar") - bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge") + bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge",cleanup_unused_imports=False) assert file1.content == content1 # language=typescript @@ -72,13 +72,11 @@ def test_unicode_move_symbol(tmpdir) -> None: file3.content == """ import { externalDep } from 'file1'; -import { bar } from "./file2"; - function baz(): string { return bar() + "🤯" + 1; } -export function barDep(): string { +function barDep(): string { return "😀"; } diff --git a/tests/unit/codegen/sdk/typescript/function/test_function_move_to_file.py b/tests/unit/codegen/sdk/typescript/function/test_function_move_to_file.py index db1b87275..61d37530e 100644 --- a/tests/unit/codegen/sdk/typescript/function/test_function_move_to_file.py +++ b/tests/unit/codegen/sdk/typescript/function/test_function_move_to_file.py @@ -83,8 +83,6 @@ def test_move_to_file_update_all_imports(tmpdir) -> None: # language=typescript EXPECTED_FILE_2_CONTENT = """ -import { externalDep } from 'file1'; - function foo() { return fooDep() + 1; } @@ -97,12 +95,11 @@ def test_move_to_file_update_all_imports(tmpdir) -> None: # language=typescript EXPECTED_FILE_3_CONTENT = """ import { externalDep } from 'file1'; -import { bar } from 'file3'; export function baz() { return bar() + 1; } -export function barDep() { +function barDep() { return 2; } @@ -112,8 +109,6 @@ def test_move_to_file_update_all_imports(tmpdir) -> None: """ # =============================== - # TODO: [!HIGH!] Self import of bar in file3 - # TODO: [medium] Why is barDep exported? # TODO: [low] Missing newline after import with get_codebase_session( @@ -181,7 +176,7 @@ def test_move_to_file_update_all_imports_include_dependencies(tmpdir) -> None: return 1; } -export function abc(): string { +function abc(): string { // dependency, gets moved return 'abc'; } @@ -210,7 +205,6 @@ def test_move_to_file_update_all_imports_include_dependencies(tmpdir) -> None: """ # =============================== - # TODO: [medium] Why is abc exported? # TODO: [low] Missing newline after import with get_codebase_session( @@ -394,8 +388,6 @@ def test_move_to_file_add_back_edge(tmpdir) -> None: # language=typescript EXPECTED_FILE_2_CONTENT = """ export { bar } from 'file3' -import { externalDep } from 'file1'; - function foo() { return fooDep() + 1; } @@ -408,13 +400,11 @@ def test_move_to_file_add_back_edge(tmpdir) -> None: # language=typescript EXPECTED_FILE_3_CONTENT = """ import { externalDep } from 'file1'; -import { bar } from 'file2'; - export function baz() { return bar() + 1; } -export function barDep() { +function barDep() { return 2; } @@ -424,9 +414,7 @@ def test_move_to_file_add_back_edge(tmpdir) -> None: """ # =============================== - # TODO: [!HIGH!] Creates circular import for bar between file2 and file3 # TODO: [medium] Missing semicolon in import on file3 - # TODO: [medium] Why did barDep get changed to export? with get_codebase_session( tmpdir=tmpdir, @@ -493,7 +481,7 @@ def test_move_to_file_add_back_edge_including_dependencies(tmpdir) -> None: return 1; } -export function abc(): string { +function abc(): string { // dependency, gets moved return 'abc'; } @@ -526,7 +514,6 @@ def test_move_to_file_add_back_edge_including_dependencies(tmpdir) -> None: # =============================== # TODO: [medium] Missing semicolon in import on file2 - # TODO: [medium] Why is abc exported? with get_codebase_session( tmpdir=tmpdir, @@ -711,8 +698,6 @@ def test_move_to_file_duplicate_dependencies(tmpdir) -> None: # language=typescript EXPECTED_FILE_2_CONTENT = """ -import { externalDep } from 'file1'; - function foo() { return fooDep() + 1; } @@ -721,21 +706,19 @@ def test_move_to_file_duplicate_dependencies(tmpdir) -> None: return 24; } -export function bar() { - return externalDep() + barDep(); +function barDep() { + return 2; } """ # language=typescript EXPECTED_FILE_3_CONTENT = """ import { externalDep } from 'file1'; -import { bar } from 'file2'; - export function baz() { return bar() + 1; } -export function barDep() { +function barDep() { return 2; } @@ -746,7 +729,6 @@ def test_move_to_file_duplicate_dependencies(tmpdir) -> None: # =============================== # TODO: [!HIGH!] Incorrect deletion of bar's import and dependency - # TODO: [medium] Why is barDep exported? with get_codebase_session( tmpdir=tmpdir, @@ -813,7 +795,7 @@ def test_move_to_file_duplicate_dependencies_include_dependencies(tmpdir) -> Non return 1; } -export function abc(): string { +function abc(): string { // dependency, gets duplicated return 'abc'; } @@ -826,6 +808,11 @@ def test_move_to_file_duplicate_dependencies_include_dependencies(tmpdir) -> Non # language=typescript EXPECTED_FILE_2_CONTENT = """ +function abc(): string { + // dependency, gets duplicated + return 'abc'; +} + export function bar(): string { // gets duplicated return abc(); @@ -848,8 +835,6 @@ def test_move_to_file_duplicate_dependencies_include_dependencies(tmpdir) -> Non """ # =============================== - # TODO: [!HIGH!] Incorrect deletion of bar's import and dependency - # TODO: [medium] Why is abc exported? # TODO: [low] Missing newline after import with get_codebase_session( @@ -1390,8 +1375,7 @@ def test_function_move_to_file_no_deps(tmpdir) -> None: # ========== [ AFTER ] ========== # language=typescript EXPECTED_FILE_1_CONTENT = """ -import { foo } from 'File2'; -export { foo } +export { foo } from 'File2'; export function bar(): number { return foo() + 1; @@ -1410,7 +1394,6 @@ def test_function_move_to_file_no_deps(tmpdir) -> None: # =============================== # TODO: [medium] Is the extra new lines here expected behavior? # TODO: [low] Missing semicolons - # TOOD: [low] Import and export should be changed to a re-export with get_codebase_session( tmpdir=tmpdir, @@ -1447,8 +1430,7 @@ def test_function_move_to_file_lower_upper_no_deps(tmpdir) -> None: # ========== [ AFTER ] ========== # language=typescript EXPECTED_FILE_1_CONTENT = """ -import { foo } from 'File1'; -export { foo } +export { foo } from 'File1'; export function bar(): number { return foo() + 1; diff --git a/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move.py b/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move.py new file mode 100644 index 000000000..ee823144f --- /dev/null +++ b/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move.py @@ -0,0 +1,1750 @@ +import platform + +import pytest + +from codegen.sdk.codebase.factory.get_session import get_codebase_session +from codegen.shared.enums.programming_language import ProgrammingLanguage + + +class TestBasicMoveToFile: + """Test basic function move functionality without imports, using multiple strategies.""" + + def test_basic_move(self, tmpdir) -> None: + """Test basic function move without imports.""" + # language=typescript + source_content = """ + export function targetFunction() { + return "Hello World"; + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=False) + + assert "targetFunction" not in source_file.content + assert "export function targetFunction" in dest_file.content + + def test_update_all_imports_basic(self, tmpdir) -> None: + """Test update_all_imports strategy updates imports in all dependent files.""" + # language=typescript + source_content = """ + export function targetFunction() { + return "Hello World"; + } + """ + + usage_content = """ + import { targetFunction } from './source'; + const value = targetFunction(); + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + "usage.ts": usage_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + usage_file = codebase.get_file("usage.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=False, strategy="update_all_imports") + + assert "targetFunction" not in source_file.content + assert "export function targetFunction" in dest_file.content + assert "import { targetFunction } from 'destination'" in usage_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_add_back_edge_basic(self, tmpdir) -> None: + """Test add_back_edge strategy - adds import in source file and re-exports the moved symbol.""" + # language=typescript + source_content = """ + export function targetFunction() { + return "Hello World"; + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=False, strategy="add_back_edge") + + assert "import { targetFunction } from 'destination'" in source_file.content + assert "export { targetFunction }" in source_file.content + assert "export function targetFunction" in dest_file.content + + def test_update_all_imports_with_dependencies(self, tmpdir) -> None: + """Test update_all_imports strategy with dependencies.""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import { helperUtil } from './utils'" not in source_file.content + assert "import { helperUtil } from './utils'" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_add_back_edge_with_dependencies(self, tmpdir) -> None: + """Test add_back_edge strategy with dependencies.""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="add_back_edge") + + assert "import { targetFunction } from 'destination'" in source_file.content + assert "import { helperUtil } from './utils'" not in source_file.content + assert "import { helperUtil } from './utils'" in dest_file.content + + +class TestMoveToFileImports: + """Test moving functions with various import scenarios.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_remove_unused_imports(self, tmpdir) -> None: + """Test that unused imports are removed when cleanup_unused_imports=True.""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports", cleanup_unused_imports=True) + + # Unused import should be removed + assert "import { otherUtil } from './other'" not in source_file.content + # Used import should move to destination + assert "import { helperUtil } from './utils'" in dest_file.content + + def test_keep_unused_imports(self, tmpdir) -> None: + """Test that unused imports are kept when cleanup_unused_imports=False.""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports", cleanup_unused_imports=False) + + # All imports should be kept in source + assert "import { helperUtil } from './utils'" in source_file.content + assert "import { otherUtil } from './other'" in source_file.content + # Used import should also be in destination + assert "import { helperUtil } from './utils'" in dest_file.content + + def test_used_imports_always_move(self, tmpdir) -> None: + """Test that used imports always move to destination regardless of remove_unused_imports flag.""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + files = { + "source.ts": source_content, + "destination.ts": "", + } + + for remove_unused in [True, False]: + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file("destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports", cleanup_unused_imports=remove_unused) + + # Used import should always move to destination + assert "import { helperUtil } from './utils'" in dest_file.content + + +class TestMoveToFileImportVariations: + """Test moving functions with various import scenarios.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_module_imports(self, tmpdir) -> None: + """Test moving a symbol that uses module imports (import * as)""" + # language=typescript + source_content = """ + import * as utils from './utils'; + import * as unused from './unused'; + + export function targetFunction() { + return utils.helperUtil("test"); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import * as utils from './utils'" not in source_file.content + assert "import * as unused from './unused'" not in source_file.content + assert "import * as utils from './utils'" in dest_file.content + + def test_move_with_side_effect_imports(self, tmpdir) -> None: + """Test moving a symbol that has side effect imports""" + # language=typescript + source_content = """ + import './styles.css'; + import './polyfills'; + import { helperUtil } from './utils'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Side effect imports should remain in source + assert "import './styles.css';" in source_file.content + assert "import './polyfills';" in source_file.content + # Used import should move + assert "import { helperUtil } from './utils'" not in source_file.content + assert "import { helperUtil } from './utils'" in dest_file.content + + def test_move_with_circular_dependencies(self, tmpdir) -> None: + """Test moving a symbol that has circular dependencies""" + # language=typescript + source_content = """ + import { helperB } from './helper-b'; + + export function targetFunction() { + return helperB(innerHelper()); + } + + function innerHelper() { + return "inner"; + } + """ + + # language=typescript + helper_b_content = """ + import { targetFunction } from './source'; + + export function helperB(value: string) { + return targetFunction(); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + "helper-b.ts": helper_b_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + helper_b_file = codebase.get_file("helper-b.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check circular dependency handling + assert "import { helperB } from './helper-b'" not in source_file.content + assert "import { helperB } from 'helper-b'" in dest_file.content + assert "import { targetFunction } from 'destination'" in helper_b_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_reexports(self, tmpdir) -> None: + """Test moving a symbol that is re-exported from multiple files""" + # language=typescript + source_content = """ + export function targetFunction() { + return "test"; + } + """ + + # language=typescript + reexport_a_content = """ + export { targetFunction } from './source'; + """ + + # language=typescript + reexport_b_content = """ + export { targetFunction as renamedFunction } from './source'; + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + "reexport-a.ts": reexport_a_content, + "reexport-b.ts": reexport_b_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + reexport_a_file = codebase.get_file("reexport-a.ts") + reexport_b_file = codebase.get_file("reexport-b.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check re-export updates + assert "export { targetFunction } from './destination'" in reexport_a_file.content + assert "export { targetFunction as renamedFunction } from './destination'" in reexport_b_file.content + + +class TestMoveToFileDecoratorsAndComments: + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_decorators(self, tmpdir) -> None: + """Test moving a symbol that has decorators""" + # language=typescript + source_content = """ + import { injectable } from 'inversify'; + import { validate } from './validators'; + + @injectable() + @validate() + export function targetFunction() { + return "test"; + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "@injectable()" not in source_file.content + assert "@validate()" not in source_file.content + assert "@injectable()" in dest_file.content + assert "@validate()" in dest_file.content + assert "import { injectable } from 'inversify'" in dest_file.content + assert "import { validate } from './validators'" in dest_file.content + + def test_move_with_jsdoc(self, tmpdir) -> None: + """Test moving a symbol with JSDoc comments""" + # language=typescript + source_content = """ + import { SomeType } from './types'; + + /** + * @param {string} value - Input value + * @returns {SomeType} Processed result + */ + export function targetFunction(value: string): SomeType { + return { value }; + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "@param {string}" not in source_file.content + assert "@returns {SomeType}" not in source_file.content + assert "@param {string}" in dest_file.content + assert "@returns {SomeType}" in dest_file.content + assert "import { SomeType } from './types'" in dest_file.content + + +class TestMoveToFileDynamicImports: + def test_move_with_dynamic_imports(self, tmpdir) -> None: + """Test moving a symbol that uses dynamic imports""" + # language=typescript + source_content = """ + export async function targetFunction() { + const { helper } = await import('./helper'); + const utils = await import('./utils'); + return helper(utils.format("test")); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import('./helper')" not in source_file.content + assert "import('./utils')" not in source_file.content + assert "import('./helper')" in dest_file.content + assert "import('./utils')" in dest_file.content + + def test_move_with_mixed_dynamic_static_imports(self, tmpdir) -> None: + """Test moving a symbol that uses both dynamic and static imports""" + # language=typescript + source_content = """ + import { baseHelper } from './base'; + + export async function targetFunction() { + const { dynamicHelper } = await import('./dynamic'); + return baseHelper(await dynamicHelper()); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import { baseHelper }" not in source_file.content + assert "import('./dynamic')" not in source_file.content + assert "import { baseHelper }" in dest_file.content + assert "import('./dynamic')" in dest_file.content + + +class TestMoveToFileNamedImports: + """Test moving functions with named imports.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_named_imports(self, tmpdir) -> None: + """Test moving a symbol that uses named imports.""" + # language=typescript + source_content = """ + import { foo, bar as alias, unused } from './module'; + + export function targetFunction() { + return foo(alias("test")); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import { foo, bar as alias" in dest_file.content + assert "unused" not in dest_file.content + assert "import { foo" not in source_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_default_and_named_imports(self, tmpdir) -> None: + """Test moving a symbol that uses both default and named imports.""" + # language=typescript + source_content = """ + import defaultHelper, { namedHelper, unusedHelper } from './helper'; + + export function targetFunction() { + return defaultHelper(namedHelper("test")); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import defaultHelper, { namedHelper }" in dest_file.content + assert "unusedHelper" not in dest_file.content + assert "defaultHelper" not in source_file.content + + +class TestMoveToFileTypeImports: + """Test moving functions with type imports.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_type_imports(self, tmpdir) -> None: + """Test moving a symbol that uses type imports.""" + # language=typescript + source_content = """ + import type { Config } from './config'; + import type DefaultType from './types'; + import type { Used as Alias, Unused } from './utils'; + + export function targetFunction(config: Config, type: DefaultType): Alias { + return { value: config.value }; + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check type imports are moved correctly + assert "import type { Config }" in dest_file.content + assert "import type DefaultType" in dest_file.content + assert "import type { Used as Alias }" in dest_file.content + assert "Unused" not in dest_file.content + # Check original file cleanup + assert "import type" not in source_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_mixed_type_value_imports(self, tmpdir) -> None: + """Test moving a symbol that uses both type and value imports.""" + # language=typescript + source_content = """ + import type { Type1, Type2 } from './types'; + import { value1, value2 } from './values'; + + export function targetFunction(t1: Type1): value1 { + return value1(t1); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check both type and value imports are handled + assert "import type { Type1 }" in dest_file.content + assert "Type2" not in dest_file.content + assert "import { value1 }" in dest_file.content + assert "value2" not in dest_file.content + + +class TestMoveToFileUsageUpdates: + """Test updating import statements in files that use the moved symbol.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_usage_file_updates(self, tmpdir) -> None: + """Test that usage files are updated correctly.""" + # language=typescript + source_content = """ + export function targetFunction() { + return "test"; + } + """ + + # language=typescript + usage_content = """ + import { targetFunction } from './source'; + import { otherFunction } from './source'; + + export function consumer() { + return targetFunction(); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + "usage.ts": usage_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + usage_file = codebase.get_file("usage.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check usage file updates + assert "import { targetFunction } from './destination'" in usage_file.content + assert "import { otherFunction } from './source'" in usage_file.content + + +class TestMoveToFileComplexScenarios: + """Test complex scenarios with multiple files and dependencies.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_complex_dependency_chain(self, tmpdir) -> None: + """Test moving a symbol with a complex chain of dependencies.""" + # language=typescript + source_content = """ + import { helperA } from './helper-a'; + import { helperB } from './helper-b'; + import type { ConfigType } from './types'; + + export function targetFunction(config: ConfigType) { + return helperA(helperB(config)); + } + """ + + # language=typescript + helper_a_content = """ + import { helperB } from './helper-b'; + export function helperA(value: string) { + return helperB(value); + } + """ + + # language=typescript + helper_b_content = """ + import type { ConfigType } from './types'; + export function helperB(config: ConfigType) { + return config.value; + } + """ + + # language=typescript + types_content = """ + export interface ConfigType { + value: string; + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + "helper-a.ts": helper_a_content, + "helper-b.ts": helper_b_content, + "types.ts": types_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check imports in destination file + assert "import { helperA } from './helper-a'" in dest_file.content + assert "import { helperB } from './helper-b'" in dest_file.content + assert "import type { ConfigType } from './types'" in dest_file.content + + # Check source file is cleaned up + assert "helperA" not in source_file.content + assert "helperB" not in source_file.content + assert "ConfigType" not in source_file.content + + +class TestMoveToFileEdgeCases: + """Test edge cases and error conditions.""" + + def test_move_with_self_reference(self, tmpdir) -> None: + """Test moving a function that references itself.""" + # language=typescript + source_content = """ + export function targetFunction(n: number): number { + if (n <= 1) return n; + return targetFunction(n - 1) + targetFunction(n - 2); + } + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check self-reference is preserved + assert "targetFunction(n - 1)" in dest_file.content + assert "targetFunction(n - 2)" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_namespace_imports(self, tmpdir) -> None: + """Test moving a symbol that uses namespace imports.""" + # language=typescript + source_content = """ + import * as ns1 from './namespace1'; + import * as ns2 from './namespace2'; + + export function targetFunction() { + return ns1.helper(ns2.config); + } + """ + + # language=typescript + namespace1_content = """ + export function helper(config: any) { + return config.value; + } + """ + + # language=typescript + namespace2_content = """ + export const config = { + value: "test" + }; + """ + + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + "source.ts": source_content, + dest_filename: dest_content, + "namespace1.ts": namespace1_content, + "namespace2.ts": namespace2_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check namespace imports are handled correctly + assert "import * as ns1 from './namespace1'" in dest_file.content + assert "import * as ns2 from './namespace2'" in dest_file.content + assert "ns1.helper" in dest_file.content + assert "ns2.config" in dest_file.content + + +class TestMoveToFileErrorConditions: + """Test error conditions and invalid moves.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_circular_dependencies(self, tmpdir) -> None: + """Test moving a symbol involved in circular dependencies.""" + # language=typescript + source_content = """ + import { helperB } from './helper-b'; + + export function targetFunction() { + return helperB(); + } + """ + + # language=typescript + helper_b_content = """ + import { targetFunction } from './source'; + + export function helperB() { + return targetFunction(); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = {source_filename: source_content, dest_filename: dest_content, "helper-b.ts": helper_b_content} + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + helper_b_file = codebase.get_file("helper-b.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check circular dependency is resolved + assert "import { targetFunction } from './destination'" in helper_b_file.content + assert "import { helperB } from './helper-b'" in dest_file.content + + +class TestMoveToFileJSXScenarios: + """Test moving JSX/TSX components and related scenarios.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_component_with_props(self, tmpdir) -> None: + """Test moving a React component with props interface.""" + # language=typescript + source_content = """ + import React from 'react'; + import type { ButtonProps } from './types'; + import { styled } from '@emotion/styled'; + + const StyledButton = styled.button` + color: blue; + `; + + export function TargetComponent({ onClick, children }: ButtonProps) { + return ( + + {children} + + ); + } + """ + + source_filename = "source.tsx" + dest_filename = "destination.tsx" + # language=typescript + dest_content = """ + """ + + files = {source_filename: source_content, dest_filename: dest_content} + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_component = source_file.get_function("TargetComponent") + target_component.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check JSX-specific imports and dependencies + assert "import React from 'react'" in dest_file.content + assert "import type { ButtonProps } from './types'" in dest_file.content + assert "import { styled } from '@emotion/styled'" in dest_file.content + assert "const StyledButton = styled.button" in dest_file.content + + +class TestMoveToFileModuleAugmentation: + """Test moving symbols with module augmentation.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_module_augmentation(self, tmpdir) -> None: + """Test moving a symbol that involves module augmentation.""" + # language=typescript + source_content = """ + declare module 'external-module' { + export interface ExternalType { + newProperty: string; + } + } + + import type { ExternalType } from 'external-module'; + + export function targetFunction(param: ExternalType) { + return param.newProperty; + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check module augmentation is handled + assert "declare module 'external-module'" in dest_file.content + assert "interface ExternalType" in dest_file.content + assert "import type { ExternalType }" in dest_file.content + + +class TestMoveToFileReExportChains: + """Test moving symbols involved in re-export chains.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_reexport_chain(self, tmpdir) -> None: + """Test moving a symbol that's re-exported through multiple files.""" + # language=typescript + source_content = """ + export function targetFunction() { + return "test"; + } + """ + + # language=typescript + barrel_a_content = """ + export { targetFunction } from './source'; + """ + + # language=typescript + barrel_b_content = """ + export * from './barrel-a'; + """ + + # language=typescript + usage_content = """ + import { targetFunction } from './barrel-b'; + + export function consumer() { + return targetFunction(); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = {source_filename: source_content, dest_filename: dest_content, "barrel-a.ts": barrel_a_content, "barrel-b.ts": barrel_b_content, "usage.ts": usage_content} + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + barrel_a_file = codebase.get_file("barrel-a.ts") + barrel_b_file = codebase.get_file("barrel-b.ts") + usage_file = codebase.get_file("usage.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check re-export chain updates + assert "export { targetFunction } from './destination'" in barrel_a_file.content + assert "export * from './barrel-a'" in barrel_b_file.content + assert "import { targetFunction } from './barrel-b'" in usage_file.content + + +class TestMoveToFileAmbientDeclarations: + """Test moving symbols with ambient declarations.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_ambient_module(self, tmpdir) -> None: + """Test moving a symbol that uses ambient module declarations.""" + # language=typescript + source_content = """ + declare module 'config' { + interface Config { + apiKey: string; + endpoint: string; + } + } + + import type { Config } from 'config'; + + export function targetFunction(config: Config) { + return fetch(config.endpoint, { + headers: { 'Authorization': config.apiKey } + }); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check ambient declarations are moved + assert "declare module 'config'" in dest_file.content + assert "interface Config" in dest_file.content + assert "import type { Config } from 'config'" in dest_file.content + + +class TestMoveToFileGenerics: + """Test moving symbols with generic type parameters.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_generic_constraints(self, tmpdir) -> None: + """Test moving a function with generic type constraints.""" + # language=typescript + source_content = """ + import { Validator, Serializable } from './types'; + + export function targetFunction>( + value: T, + validator: U + ): T { + return validator.validate(value); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + assert "import { Validator, Serializable }" not in source_file.content + assert "import { Validator, Serializable } from './types'" in dest_file.content + + +class TestMoveToFileDecoratorFactories: + """Test moving symbols with decorator factories.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_decorator_factories(self, tmpdir) -> None: + """Test moving a function that uses decorator factories.""" + # language=typescript + source_content = """ + import { createDecorator } from './decorator-factory'; + import type { Options } from './types'; + + const customDecorator = createDecorator({ timeout: 1000 }); + + @customDecorator + export function targetFunction() { + return new Promise(resolve => setTimeout(resolve, 1000)); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check decorator factory and its dependencies are moved + assert "import { createDecorator }" in dest_file.content + assert "import type { Options }" in dest_file.content + assert "const customDecorator = createDecorator" in dest_file.content + + +class TestMoveToFileDefaultExports: + """Test moving symbols with default exports and re-exports.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_default_export(self, tmpdir) -> None: + """Test moving a default exported function.""" + # language=typescript + source_content = """ + import { helper } from './helper'; + + export default function targetFunction() { + return helper(); + } + """ + + # language=typescript + usage_content = """ + import targetFunction from './source'; + import { default as renamed } from './source'; + + export const result = targetFunction(); + export const aliased = renamed(); + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = {source_filename: source_content, dest_filename: dest_content, "usage.ts": usage_content} + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + usage_file = codebase.get_file("usage.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Check default export handling + assert "import targetFunction from './destination'" in usage_file.content + assert "import { default as renamed } from './destination'" in usage_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_multiline_imports(self, tmpdir) -> None: + """Test removing unused imports from multiline import statements""" + # language=typescript + source_content = """ + import { + helperUtil, + formatUtil, + parseUtil, + unusedUtil + } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + const formatted = formatUtil(helperUtil("test")); + return parseUtil(formatted); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify only used imports were moved + assert "unusedUtil" not in source_file.content + assert "otherUtil" not in source_file.content + assert "helperUtil" in dest_file.content + assert "formatUtil" in dest_file.content + assert "parseUtil" in dest_file.content + assert "unusedUtil" not in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_aliased_imports(self, tmpdir) -> None: + """Test removing unused imports with aliases""" + # language=typescript + source_content = """ + import { helperUtil as helper } from './utils'; + import { formatUtil as fmt, parseUtil as parse } from './formatters'; + import { validateUtil as validate } from './validators'; + + export function targetFunction() { + return helper(fmt("test")); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify only used aliased imports were moved + assert "helper" not in source_file.content + assert "fmt" not in source_file.content + assert "parse" not in source_file.content + assert "validate" in source_file.content + assert "helper" in dest_file.content + assert "fmt" in dest_file.content + assert "parse" not in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_back_edge_with_import_retention(self, tmpdir) -> None: + """Test back edge strategy retains necessary imports""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="add_back_edge", cleanup_unused_imports=True) + + # Source file should have import from new location but keep originals + assert "import { targetFunction } from './destination'" in source_file.content + assert "import { helperUtil } from './utils'" in source_file.content + assert "import { otherUtil } from './other'" in source_file.content + # Destination should have required imports + assert "import { helperUtil } from './utils'" in dest_file.content + + +class TestMoveToFileStrategies: + """Test different move strategies and their behaviors.""" + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_update_all_imports_strategy(self, tmpdir) -> None: + """Test update_all_imports strategy behavior""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports", cleanup_unused_imports=True) + + assert "import { helperUtil } from './utils'" not in source_file.content + assert "import { otherUtil } from './other'" not in source_file.content + assert "import { helperUtil } from './utils'" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_back_edge_strategy(self, tmpdir) -> None: + """Test back edge strategy behavior""" + # language=typescript + source_content = """ + import { helperUtil } from './utils'; + import { otherUtil } from './other'; + + export function targetFunction() { + return helperUtil("test"); + } + """ + + source_filename = "source.ts" + dest_filename = "destination.ts" + # language=typescript + dest_content = """ + """ + + files = { + source_filename: source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file(source_filename) + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="add_back_edge", cleanup_unused_imports=True) + + # Source file should have import from new location + assert "import { targetFunction } from './destination'" in source_file.content + assert "import { helperUtil } from './utils'" in source_file.content + assert "import { otherUtil } from './other'" in source_file.content + # Destination should have required imports + assert "import { helperUtil } from './utils'" in dest_file.content + + def test_move_with_absolute_imports(self, tmpdir) -> None: + """Test moving a symbol that uses absolute imports""" + # language=typescript + source_content = """ + import { helperUtil } from '@/utils/helpers'; + import { formatUtil } from '/src/utils/format'; + import { configUtil } from '~/config'; + + export function targetFunction() { + return helperUtil(formatUtil(configUtil.getValue())); + } + """ + + dest_filename = "destination.ts" + dest_content = "" + + files = { + "source.ts": source_content, + dest_filename: dest_content, + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("source.ts") + dest_file = codebase.get_file(dest_filename) + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify absolute imports are preserved + assert "import { helperUtil } from '@/utils/helpers'" in dest_file.content + assert "import { formatUtil } from '/src/utils/format'" in dest_file.content + assert "import { configUtil } from '~/config'" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_complex_relative_paths(self, tmpdir) -> None: + """Test moving a symbol that uses complex relative paths""" + # language=typescript + source_content = """ + import { helperA } from '../../../utils/helpers'; + import { helperB } from '../../../../shared/utils'; + import { helperC } from './local/helper'; + + export function targetFunction() { + return helperA(helperB(helperC())); + } + """ + + files = { + "src/features/auth/components/source.ts": source_content, + "src/features/user/services/destination.ts": "", + "src/utils/helpers.ts": "export const helperA = (x) => x;", + "shared/utils.ts": "export const helperB = (x) => x;", + "src/features/auth/components/local/helper.ts": "export const helperC = () => 'test';", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("src/features/auth/components/source.ts") + dest_file = codebase.get_file("src/features/user/services/destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify relative paths are correctly updated based on new file location + assert "import { helperA } from '../../utils/helpers'" in dest_file.content + assert "import { helperB } from '../../../../shared/utils'" in dest_file.content + assert "import { helperC } from '../../auth/components/local/helper'" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_with_mixed_import_styles(self, tmpdir) -> None: + """Test moving a symbol that uses mixed import styles""" + # language=typescript + source_content = """ + import defaultHelper from '@/helpers/default'; + import * as utils from '~/utils'; + import { namedHelper as aliasedHelper } from '../shared/helpers'; + import type { HelperType } from './types'; + const dynamicHelper = await import('./dynamic-helper'); + + export function targetFunction(): HelperType { + return defaultHelper( + utils.helper( + aliasedHelper( + dynamicHelper.default() + ) + ) + ); + } + """ + + files = { + "src/features/source.ts": source_content, + "src/services/destination.ts": "", + "src/helpers/default.ts": "export default (x) => x;", + "lib/utils.ts": "export const helper = (x) => x;", + "src/shared/helpers.ts": "export const namedHelper = (x) => x;", + "src/features/types.ts": "export type HelperType = string;", + "src/features/dynamic-helper.ts": "export default () => 'test';", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("src/features/source.ts") + dest_file = codebase.get_file("src/services/destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify different import styles are handled correctly + assert "import defaultHelper from '@/helpers/default'" in dest_file.content + assert "import * as utils from '~/utils'" in dest_file.content + assert "import { namedHelper as aliasedHelper } from '../shared/helpers'" in dest_file.content + assert "import type { HelperType } from '../features/types'" in dest_file.content + assert "const dynamicHelper = await import('../features/dynamic-helper')" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_between_monorepo_packages(self, tmpdir) -> None: + """Test moving a symbol between different packages in a monorepo""" + # language=typescript + source_content = """ + import { sharedUtil } from '@myorg/shared'; + import { helperUtil } from '@myorg/utils'; + import { localUtil } from './utils'; + + export function targetFunction() { + return sharedUtil(helperUtil(localUtil())); + } + """ + + files = { + "packages/package-a/src/source.ts": source_content, + "packages/package-b/src/destination.ts": "", + "packages/shared/src/index.ts": "export const sharedUtil = (x) => x;", + "packages/utils/src/index.ts": "export const helperUtil = (x) => x;", + "packages/package-a/src/utils.ts": "export const localUtil = () => 'test';", + "packages/package-a/package.json": '{"name": "@myorg/package-a"}', + "packages/package-b/package.json": '{"name": "@myorg/package-b"}', + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("packages/package-a/src/source.ts") + dest_file = codebase.get_file("packages/package-b/src/destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify package imports are handled correctly + assert "import { sharedUtil } from '@myorg/shared'" in dest_file.content + assert "import { helperUtil } from '@myorg/utils'" in dest_file.content + assert "import { localUtil } from '@myorg/package-a/src/utils'" in dest_file.content + + @pytest.mark.skip(reason="This test or related implementation needs work.") + def test_move_between_different_depths(self, tmpdir) -> None: + """Test moving a symbol between files at different directory depths""" + # language=typescript + source_content = """ + import { helperA } from './helper'; + import { helperB } from '../utils/helper'; + import { helperC } from '../../shared/helper'; + + export function targetFunction() { + return helperA(helperB(helperC())); + } + """ + + files = { + "src/features/auth/source.ts": source_content, + "src/features/auth/helper.ts": "export const helperA = (x) => x;", + "src/features/utils/helper.ts": "export const helperB = (x) => x;", + "src/shared/helper.ts": "export const helperC = () => 'test';", + "lib/services/destination.ts": "", + } + + with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: + source_file = codebase.get_file("src/features/auth/source.ts") + dest_file = codebase.get_file("lib/services/destination.ts") + + target_function = source_file.get_function("targetFunction") + target_function.move_to_file(dest_file, include_dependencies=True, strategy="update_all_imports") + + # Verify imports are updated for new directory depth + assert "import { helperA } from '../../src/features/auth/helper'" in dest_file.content + assert "import { helperB } from '../../src/features/utils/helper'" in dest_file.content + assert "import { helperC } from '../../src/shared/helper'" in dest_file.content + + +class TestMoveToFileFileSystem: + """Test moving functions with different file system considerations.""" + + @pytest.mark.skipif(condition=platform.system() != "Linux", reason="Only works on case-sensitive file systems") + def test_function_move_to_file_lower_upper(self, tmpdir) -> None: + # language=typescript + content1 = """ +export function foo(): number { + return bar() + 1; +} + +export function bar(): number { + return foo() + 1; +} + """ + with get_codebase_session(tmpdir, files={"file1.ts": content1}, programming_language=ProgrammingLanguage.TYPESCRIPT) as codebase: + file1 = codebase.get_file("file1.ts") + foo = file1.get_function("foo") + bar = file1.get_function("bar") + assert bar in foo.dependencies + assert foo in bar.dependencies + + file2 = codebase.create_file("File1.ts", "") + foo.move_to_file(file2, include_dependencies=True, strategy="add_back_edge") + + # language=typescript + assert ( + file2.content.strip() + == """ +export function bar(): number { + return foo() + 1; +} + +export function foo(): number { + return bar() + 1; +} + """.strip() + ) + assert file1.content.strip() == "export { bar } from 'File1'\nexport { foo } from 'File1'" + + @pytest.mark.skipif(condition=platform.system() != "Linux", reason="Only works on case-sensitive file systems") + def test_function_move_to_file_lower_upper_no_deps(self, tmpdir) -> None: + # language=typescript + content1 = """ +export function foo(): number { + return bar() + 1; +} + +export function bar(): number { + return foo() + 1; +} + """ + with get_codebase_session(tmpdir, files={"file1.ts": content1}, programming_language=ProgrammingLanguage.TYPESCRIPT) as codebase: + file1 = codebase.get_file("file1.ts") + foo = file1.get_function("foo") + bar = file1.get_function("bar") + assert bar in foo.dependencies + assert foo in bar.dependencies + + file2 = codebase.create_file("File1.ts", "") + foo.move_to_file(file2, include_dependencies=False, strategy="add_back_edge") + + # language=typescript + assert ( + file1.content.strip() + == """export { foo } from 'File1'; + +export function bar(): number { + return foo() + 1; +}""" + ) + # language=typescript + assert ( + file2.content.strip() + == """ +import { bar } from 'file1'; + + +export function foo(): number { + return bar() + 1; +} + """.strip() + ) diff --git a/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move_tsx_to_file.py b/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move_tsx_to_file.py index d2c3e8484..ec3524e42 100644 --- a/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move_tsx_to_file.py +++ b/tests/unit/codegen/sdk/typescript/move_symbol_to_file/test_move_tsx_to_file.py @@ -1,3 +1,5 @@ +import pytest + from codegen.sdk.codebase.factory.get_session import get_codebase_session from codegen.shared.enums.programming_language import ProgrammingLanguage @@ -63,7 +65,7 @@ def test_move_component_with_dependencies(tmpdir) -> None: # Verify ComponentB move assert "const ComponentB" not in src_file.content - assert "import { ComponentB } from 'dst'" in src_file.content + assert "export { ComponentB } from 'dst'" in src_file.content assert "const ComponentB = () => {" in dst_file.content assert "export { ComponentB }" in src_file.content @@ -72,11 +74,12 @@ def test_move_component_with_dependencies(tmpdir) -> None: assert "export { ComponentD } from 'dst'" in src_file.content +@pytest.mark.skip(reason="This test is failing because of the way we handle re-exports. Address in CG-10686") def test_remove_unused_exports(tmpdir) -> None: """Tests removing unused exports when moving components between files""" - src_filename = "Component.tsx" + # ========== [ BEFORE ] ========== # language=typescript jsx - src_content = """ + SRC_CONTENT = """ export default function MainComponent() { const [state, setState] = useState() return (
@@ -116,9 +119,8 @@ def test_remove_unused_exports(tmpdir) -> None: ) } """ - adj_filename = "adjacent.tsx" # language=typescript jsx - adj_content = """ + ADJ_CONTENT = """ import MainComponent from 'Component' import { SharedComponent } from 'Component' import { StateComponent } from 'utils' @@ -127,26 +129,79 @@ def test_remove_unused_exports(tmpdir) -> None: return () } """ - misc_filename = "misc.tsx" # language=typescript jsx - misc_content = """ + MISC_CONTENT = """ export { UnusedComponent } from 'Component' function Helper({ props }: HelperProps) {} export { Helper } """ - import_filename = "import.tsx" # language=typescript jsx - import_content = """ + IMPORT_CONTENT = """ import { UnusedComponent } from 'misc' """ - files = {src_filename: src_content, adj_filename: adj_content, misc_filename: misc_content, import_filename: import_content} + # ========== [ AFTER ] ========== + # language=typescript jsx + EXPECTED_SRC_CONTENT = """ +import { SubComponent } from 'new'; + +export default function MainComponent() { + const [state, setState] = useState() + return (
+
+ +
+
) +} + +export function UnusedComponent({ props }: UnusedProps) { + return ( +
Unused
+ ) +} +""" + # language=typescript jsx + EXPECTED_NEW_CONTENT = """ +export function SubComponent({ props }: SubComponentProps) { + return ( + + ) +} + +function HelperComponent({ props }: HelperComponentProps) { + return ( + + ) +} + +export function SharedComponent({ props }: SharedComponentProps) { + return ( +
+ ) +} +""" + # language=typescript jsx + EXPECTED_ADJ_CONTENT = """ +import MainComponent from 'Component' +import { SharedComponent } from 'new' +import { StateComponent } from 'utils' + +function Container(props: ContainerProps) { + return () +} +""" + # language=typescript jsx + EXPECTED_MISC_CONTENT = """ +function Helper({ props }: HelperProps) {} +""" + + files = {"Component.tsx": SRC_CONTENT, "adjacent.tsx": ADJ_CONTENT, "misc.tsx": MISC_CONTENT, "import.tsx": IMPORT_CONTENT} with get_codebase_session(tmpdir=tmpdir, programming_language=ProgrammingLanguage.TYPESCRIPT, files=files) as codebase: - src_file = codebase.get_file(src_filename) - adj_file = codebase.get_file(adj_filename) - misc_file = codebase.get_file(misc_filename) + src_file = codebase.get_file("Component.tsx") + adj_file = codebase.get_file("adjacent.tsx") + misc_file = codebase.get_file("misc.tsx") new_file = codebase.create_file("new.tsx") sub_component = src_file.get_symbol("SubComponent") @@ -159,20 +214,7 @@ def test_remove_unused_exports(tmpdir) -> None: src_file.remove_unused_exports() misc_file.remove_unused_exports() - # Verify exports in new file - assert "export function SubComponent" in new_file.content - assert "function HelperComponent" in new_file.content - assert "export function HelperComponent" not in new_file.content - assert "export function SharedComponent" in new_file.content - - # Verify imports updated - assert "import { SharedComponent } from 'new'" in adj_file.content - - # Verify original file exports - assert "export default function MainComponent()" in src_file.content - assert "function UnusedComponent" in src_file.content - assert "export function UnusedComponent" not in src_file.content - - # Verify misc file exports cleaned up - assert "export { Helper }" not in misc_file.content - assert "export { UnusedComponent } from 'Component'" not in misc_file.content + assert src_file.content.strip() == EXPECTED_SRC_CONTENT.strip() + assert new_file.content.strip() == EXPECTED_NEW_CONTENT.strip() + assert adj_file.content.strip() == EXPECTED_ADJ_CONTENT.strip() + assert misc_file.content.strip() == EXPECTED_MISC_CONTENT.strip() diff --git a/tests/unit/codegen/sdk/typescript/tsx/test_tsx_edit.py b/tests/unit/codegen/sdk/typescript/tsx/test_tsx_edit.py index 6f21af839..a7147bf3a 100644 --- a/tests/unit/codegen/sdk/typescript/tsx/test_tsx_edit.py +++ b/tests/unit/codegen/sdk/typescript/tsx/test_tsx_edit.py @@ -333,7 +333,7 @@ def test_tsx_move_component(tmpdir) -> None: ctx.commit_transactions() assert "export function FooBar" in new_file.content - assert "export function MyFooBar" in new_file.content + assert "function MyFooBar" in new_file.content assert "import { FooBar } from 'new'" in original_file.content assert "import { MyFooBar } from 'new'" not in original_file.content diff --git a/tests/unit/codegen/sdk/typescript/tsx/test_tsx_parsing.py b/tests/unit/codegen/sdk/typescript/tsx/test_tsx_parsing.py index af2f32446..813102927 100644 --- a/tests/unit/codegen/sdk/typescript/tsx/test_tsx_parsing.py +++ b/tests/unit/codegen/sdk/typescript/tsx/test_tsx_parsing.py @@ -105,7 +105,7 @@ def test_tsx_file_type_validation(tmpdir) -> None: test_component.move_to_file(tsx_file) - assert "export function TestComponent" in tsx_file.content + assert "function TestComponent" in tsx_file.content def test_jsx_element_attributes(tmpdir) -> None: diff --git a/tests/unit/skills/implementations/guides/organize-your-codebase.py b/tests/unit/skills/implementations/guides/organize-your-codebase.py index 5827d2ca5..d2e914bd2 100644 --- a/tests/unit/skills/implementations/guides/organize-your-codebase.py +++ b/tests/unit/skills/implementations/guides/organize-your-codebase.py @@ -416,7 +416,7 @@ def my_symbol(): SkillTestCaseTSFile( input="", output=""" -export function dependencyFunction() { +function dependencyFunction() { console.log("I'm a dependency"); } From 89de94bbadbab4b32a8bbaebab70756d1fbd1ee2 Mon Sep 17 00:00:00 2001 From: tomcodgen <191515280+tomcodgen@users.noreply.github.com> Date: Sat, 15 Mar 2025 04:50:56 +0000 Subject: [PATCH 2/6] Automated pre-commit update --- .../sdk/codebase/transaction_manager.py | 14 +- src/codegen/sdk/core/file.py | 1 - src/codegen/sdk/core/import_resolution.py | 6 +- src/codegen/sdk/core/symbol.py | 166 +++++++++--------- src/codegen/sdk/typescript/symbol.py | 98 +++++------ .../sdk/python/file/test_file_unicode.py | 2 +- .../function/test_function_move_to_file.py | 52 +++--- .../sdk/typescript/file/test_file_remove.py | 2 +- .../sdk/typescript/file/test_file_unicode.py | 2 +- 9 files changed, 167 insertions(+), 176 deletions(-) diff --git a/src/codegen/sdk/codebase/transaction_manager.py b/src/codegen/sdk/codebase/transaction_manager.py index 6aa0a6598..87e938a1c 100644 --- a/src/codegen/sdk/codebase/transaction_manager.py +++ b/src/codegen/sdk/codebase/transaction_manager.py @@ -290,20 +290,20 @@ def get_transactions_at_range(self, file_path: Path, start_byte: int, end_byte: return matching_transactions - def get_transaction_containing_range(self, file_path: Path, start_byte: int, end_byte: int, transaction_order: TransactionPriority | None = None) -> Transaction|None: + def get_transaction_containing_range(self, file_path: Path, start_byte: int, end_byte: int, transaction_order: TransactionPriority | None = None) -> Transaction | None: """Returns the nearest transaction that includes the range specified given the filtering criteria.""" if file_path not in self.queued_transactions: return None - smallest_difference=math.inf - best_fit_transaction=None + smallest_difference = math.inf + best_fit_transaction = None for t in self.queued_transactions[file_path]: - if t.start_byte <= start_byte and t.end_byte>=end_byte: + if t.start_byte <= start_byte and t.end_byte >= end_byte: if transaction_order is None or t.transaction_order == transaction_order: - smallest_difference = min(smallest_difference,abs(t.start_byte-start_byte)+abs(t.end_byte-end_byte)) - if smallest_difference==0: + smallest_difference = min(smallest_difference, abs(t.start_byte - start_byte) + abs(t.end_byte - end_byte)) + if smallest_difference == 0: return t - best_fit_transaction=t + best_fit_transaction = t return best_fit_transaction def _get_conflicts(self, transaction: Transaction) -> list[Transaction]: diff --git a/src/codegen/sdk/core/file.py b/src/codegen/sdk/core/file.py index 5da6ec980..e5af34ef9 100644 --- a/src/codegen/sdk/core/file.py +++ b/src/codegen/sdk/core/file.py @@ -943,7 +943,6 @@ def remove_unused_exports(self) -> None: None """ - def remove_unused_imports(self) -> None: # Process each import statement for import_stmt in self.imports: diff --git a/src/codegen/sdk/core/import_resolution.py b/src/codegen/sdk/core/import_resolution.py index 6fdd2adfc..9f5fa8efa 100644 --- a/src/codegen/sdk/core/import_resolution.py +++ b/src/codegen/sdk/core/import_resolution.py @@ -232,8 +232,6 @@ def usage_is_ascertainable(self) -> bool: return False return True - - @reader def is_wildcard_import(self) -> bool: """Returns True if the import symbol is a wildcard import. @@ -249,13 +247,13 @@ def is_wildcard_import(self) -> bool: @reader def is_sideffect_import(self) -> bool: - #Maybe better name for this + # Maybe better name for this """Determines if this is a sideffect. Returns: bool: True if this is a sideffect import, False otherwise """ - return self.import_type==ImportType.SIDE_EFFECT + return self.import_type == ImportType.SIDE_EFFECT @property @abstractmethod diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index 81bdbe6ee..2a10555bb 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -272,7 +272,7 @@ def move_to_file( file: SourceFile, include_dependencies: bool = True, strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", - cleanup_unused_imports: bool = True + cleanup_unused_imports: bool = True, ) -> None: """Moves the given symbol to a new file and updates its imports and references. @@ -295,12 +295,11 @@ def move_to_file( self._move_to_file(file, encountered_symbols, include_dependencies, strategy, cleanup_unused_imports) @noapidoc - def _move_dependencies(self, - file: SourceFile, - encountered_symbols: set[Symbol | Import], - strategy:Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports" - ): + def _move_dependencies( + self, file: SourceFile, encountered_symbols: set[Symbol | Import], strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports" + ): from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: if dep in encountered_symbols: continue @@ -318,13 +317,14 @@ def _move_dependencies(self, # =====[ Imports - copy over ]===== elif isinstance(dep, Import): if dep.imported_symbol: - file.add_import(imp=dep.imported_symbol, alias=dep.alias.source,import_type=dep.import_type) + file.add_import(imp=dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type) else: file.add_import(imp=dep.source) @noapidoc - def _update_dependencies_on_move(self,file:SourceFile): + def _update_dependencies_on_move(self, file: SourceFile): from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: # =====[ Symbols - add back edge ]===== if isinstance(dep, Symbol) and dep.is_top_level: @@ -336,48 +336,48 @@ def _update_dependencies_on_move(self,file:SourceFile): file.add_import(imp=dep.source) @noapidoc - def _execute_post_move_correction_strategy(self, - file:SourceFile, - encountered_symbols: set[Symbol | Import], - strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], - ): - from codegen.sdk.core.import_resolution import Import - import_line = self.get_import_string(module=file.import_module_name) - is_used_in_file = any( - usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols - for usage in self.symbol_usages - ) - match strategy: - case "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - case "add_back_edge": - if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.file.add_import(imp=import_line) - - # Delete the original symbol - self.remove() - case "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: - # Add updated import - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) - usage.usage_symbol.file.add_import(imp=import_line) - - # Add the import to the original file - if is_used_in_file: - self.file.add_import(imp=import_line) - # Delete the original symbol + def _execute_post_move_correction_strategy( + self, + file: SourceFile, + encountered_symbols: set[Symbol | Import], + strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], + ): + from codegen.sdk.core.import_resolution import Import + + import_line = self.get_import_string(module=file.import_module_name) + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) + match strategy: + case "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): self.remove() + case "add_back_edge": + if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.file.add_import(imp=import_line) + + # Delete the original symbol + self.remove() + case "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: + # Add updated import + usage.usage_symbol.file.add_import(import_line) + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(imp=import_line) + + # Add the import to the original file + if is_used_in_file: + self.file.add_import(imp=import_line) + # Delete the original symbol + self.remove() + @noapidoc def _move_to_file( self, @@ -385,75 +385,75 @@ def _move_to_file( encountered_symbols: set[Symbol | Import], include_dependencies: bool = True, strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", - cleanup_unused_imports: bool = True + cleanup_unused_imports: bool = True, ) -> tuple[NodeId, NodeId]: """Helper recursive function for `move_to_file`""" # =====[ Arg checking ]===== if file == self.file: return file.file_node_id, self.node_id - #This removes any imports in the file we're moving the symbol to + # This removes any imports in the file we're moving the symbol to if imp := file.get_import(self.name): encountered_symbols.add(imp) imp.remove() - #This handles the new file + # This handles the new file # =====[ Move over dependencies recursively ]===== if include_dependencies: - self._move_dependencies(file,encountered_symbols,strategy) + self._move_dependencies(file, encountered_symbols, strategy) else: self._update_dependencies_on_move(file) # =====[ Make a new symbol in the new file ]===== should_export = False - if hasattr(self, "is_exported") and (self.is_exported or [ - usage for usage in self.usages - if usage.usage_symbol - not in encountered_symbols and not self.transaction_manager.get_transaction_containing_range( - usage.usage_symbol.file.path,usage.usage_symbol.start_byte,usage.usage_symbol.end_byte,TransactionPriority.Remove - ) - ]): - should_export=True - file.add_symbol(self,should_export=should_export) + if hasattr(self, "is_exported") and ( + self.is_exported + or [ + usage + for usage in self.usages + if usage.usage_symbol not in encountered_symbols + and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) + ] + ): + should_export = True + file.add_symbol(self, should_export=should_export) # =====[ Checks if symbol is used in original file ]===== # Takes into account that it's dependencies will be moved from codegen.sdk.core.import_resolution import Import - self._execute_post_move_correction_strategy(file,encountered_symbols,strategy) + self._execute_post_move_correction_strategy(file, encountered_symbols, strategy) # =====[ Remove any imports that are no longer used ]===== if cleanup_unused_imports: for dep in self.dependencies: - if strategy!='duplicate_dependencies': - other_usages= [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] + if strategy != "duplicate_dependencies": + other_usages = [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] else: - other_usages= [usage.usage_symbol for usage in dep.usages] - if isinstance(dep,Import): + other_usages = [usage.usage_symbol for usage in dep.usages] + if isinstance(dep, Import): dep.remove_if_unused() if not other_usages: - #Clean up forward... + # Clean up forward... dep.remove() - elif isinstance(dep,Symbol): + elif isinstance(dep, Symbol): usages_in_file = [ - symb for symb in other_usages - if symb.file==self.file and not self.transaction_manager.get_transaction_containing_range( - symb.file.path,symb.start_byte,symb.end_byte,TransactionPriority.Remove - ) - ] - if self.transaction_manager.get_transaction_containing_range( - dep.file.path,dep.start_byte,dep.end_byte,transaction_order=TransactionPriority.Remove): - if not usages_in_file and strategy!="add_back_edge": - #We are going to assume there is only one such import - if imp_list :=[import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: + symb + for symb in other_usages + if symb.file == self.file and not self.transaction_manager.get_transaction_containing_range(symb.file.path, symb.start_byte, symb.end_byte, TransactionPriority.Remove) + ] + if self.transaction_manager.get_transaction_containing_range(dep.file.path, dep.start_byte, dep.end_byte, transaction_order=TransactionPriority.Remove): + if not usages_in_file and strategy != "add_back_edge": + # We are going to assume there is only one such import + if imp_list := [import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: if insert_import_list := [ - transaction for transaction in self.transaction_manager.queued_transactions[self.file.path] - if imp_list[0] and transaction.new_content and imp_list[0] in transaction.new_content and transaction.transaction_order==TransactionPriority.Insert - ]: + transaction + for transaction in self.transaction_manager.queued_transactions[self.file.path] + if imp_list[0] and transaction.new_content and imp_list[0] in transaction.new_content and transaction.transaction_order == TransactionPriority.Insert + ]: self.transaction_manager.queued_transactions[self.file.path].remove(insert_import_list[0]) self.file._pending_imports.remove(imp_list[0]) - @property @reader @noapidoc diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index 813d84490..e323222c3 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -255,7 +255,7 @@ def has_semicolon(self) -> bool: @noapidoc @override - def _update_dependencies_on_move(self,file:SourceFile): + def _update_dependencies_on_move(self, file: SourceFile): for dep in self.dependencies: if isinstance(dep, Assignment): msg = "Assignment not implemented yet" @@ -277,55 +277,56 @@ def _update_dependencies_on_move(self,file:SourceFile): @noapidoc @override - def _execute_post_move_correction_strategy(self, - file:SourceFile, - encountered_symbols: set[Symbol | Import], - strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], - ): - import_line = self.get_import_string(module=file.import_module_name) - # =====[ Checks if symbol is used in original file ]===== - # Takes into account that it's dependencies will be moved - is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) - - match strategy: - case "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL for usage in self.symbol_usages) - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - case "add_back_edge": - if is_used_in_file: - back_edge_line = import_line - if self.is_exported: - back_edge_line=back_edge_line.replace('import','export') - self.file.add_import(back_edge_line) - elif self.is_exported: - module_name = file.name - self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") - # Delete the original symbol + def _execute_post_move_correction_strategy( + self, + file: SourceFile, + encountered_symbols: set[Symbol | Import], + strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], + ): + import_line = self.get_import_string(module=file.import_module_name) + # =====[ Checks if symbol is used in original file ]===== + # Takes into account that it's dependencies will be moved + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) + + match strategy: + case "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL for usage in self.symbol_usages) + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): self.remove() - case "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, TSImport): - # Add updated import - if usage.usage_symbol.resolved_symbol is not None and usage.usage_symbol.resolved_symbol.node_type == NodeType.SYMBOL and usage.usage_symbol.resolved_symbol == self: - if usage.usage_symbol.file!=file: - # Just remove if the dep is now going to the file - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) + case "add_back_edge": + if is_used_in_file: + back_edge_line = import_line + if self.is_exported: + back_edge_line = back_edge_line.replace("import", "export") + self.file.add_import(back_edge_line) + elif self.is_exported: + module_name = file.name + self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") + # Delete the original symbol + self.remove() + case "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, TSImport): + # Add updated import + if usage.usage_symbol.resolved_symbol is not None and usage.usage_symbol.resolved_symbol.node_type == NodeType.SYMBOL and usage.usage_symbol.resolved_symbol == self: + if usage.usage_symbol.file != file: + # Just remove if the dep is now going to the file usage.usage_symbol.file.add_import(import_line) - - if is_used_in_file: - self.file.add_import(import_line) - # Delete the original symbol - self.remove() + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(import_line) + + if is_used_in_file: + self.file.add_import(import_line) + # Delete the original symbol + self.remove() def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter | None, level: int) -> str: """Converts a PropType definition to its TypeScript equivalent.""" @@ -334,7 +335,6 @@ def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter if prop_type.source in type_map: return type_map[prop_type.source] - if isinstance(prop_type, ChainedAttribute): if prop_type.attribute.source == "node": return "T" diff --git a/tests/unit/codegen/sdk/python/file/test_file_unicode.py b/tests/unit/codegen/sdk/python/file/test_file_unicode.py index 6509d5d77..0792c266e 100644 --- a/tests/unit/codegen/sdk/python/file/test_file_unicode.py +++ b/tests/unit/codegen/sdk/python/file/test_file_unicode.py @@ -39,7 +39,7 @@ def baz(): file3 = codebase.get_file("file3.py") bar = file2.get_function("bar") - bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge",cleanup_unused_imports=False) + bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge", cleanup_unused_imports=False) assert file1.content == content1 # language=python diff --git a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py index 93059445f..cefb0c802 100644 --- a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py +++ b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py @@ -368,6 +368,7 @@ def bar(): assert file2.content.strip() == EXPECTED_FILE_2_CONTENT.strip() assert file3.content.strip() == EXPECTED_FILE_3_CONTENT.strip() + def test_move_to_file_add_back_edge_external_use(tmpdir) -> None: # ========== [ BEFORE ] ========== # language=python @@ -407,7 +408,6 @@ def bla(): return bar() + 1 """ - # ========== [ AFTER ] ========== # language=python EXPECTED_FILE_1_CONTENT = """ @@ -455,7 +455,6 @@ def bla(): "file2.py": FILE_2_CONTENT, "file3.py": FILE_3_CONTENT, "file4.py": FILE_4_CONTENT, - }, ) as codebase: file1 = codebase.get_file("file1.py") @@ -1480,7 +1479,6 @@ class ExtendedConfig(Config): assert file2_types.content.strip() == EXPECTED_FILE_2_TYPES_CONTENT.strip() - def test_move_to_file_decorators(tmpdir) -> None: # ========== [ BEFORE ] ========== # language=python @@ -1500,13 +1498,12 @@ def test_func(): """ with get_codebase_session( - tmpdir=tmpdir, - files={ - "file1.py": FILE_1_CONTENT, - "file2.py": FILE_2_CONTENT, - }, - ) as codebase: - + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") @@ -1546,10 +1543,10 @@ def boo(): # language=python FILE_2_CONTENT = "NO_MOVE_FILE_2 = 6" - FILE_1_EXPECTED =""" + FILE_1_EXPECTED = """ NO_MOVE=2 """ - FILE_2_EXPECTED =""" + FILE_2_EXPECTED = """ from test.foo import TEST NO_MOVE_FILE_2 = 6 @@ -1573,13 +1570,12 @@ def boo(): """ with get_codebase_session( - tmpdir=tmpdir, - files={ - "file1.py": FILE_1_CONTENT, - "file2.py": FILE_2_CONTENT, - }, - ) as codebase: - + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") @@ -1597,7 +1593,6 @@ def boo(): assert file2.source.strip() == FILE_2_EXPECTED.strip() - def test_move_to_file_multiple_same_transaction_partial(tmpdir) -> None: # language=python FILE_1_CONTENT = """ @@ -1626,7 +1621,7 @@ def boo(): # language=python FILE_2_CONTENT = "NO_MOVE_FILE_2 = 6" - FILE_1_EXPECTED =""" + FILE_1_EXPECTED = """ from file2 import useful NO_MOVE=2 @@ -1634,7 +1629,7 @@ def boo(): print(6) useful() """ - FILE_2_EXPECTED =""" + FILE_2_EXPECTED = """ from test.foo import TEST NO_MOVE_FILE_2 = 6 @@ -1654,13 +1649,12 @@ def bar(): """ with get_codebase_session( - tmpdir=tmpdir, - files={ - "file1.py": FILE_1_CONTENT, - "file2.py": FILE_2_CONTENT, - }, - ) as codebase: - + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + }, + ) as codebase: file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") diff --git a/tests/unit/codegen/sdk/typescript/file/test_file_remove.py b/tests/unit/codegen/sdk/typescript/file/test_file_remove.py index 0fb07d96e..39b8932ee 100644 --- a/tests/unit/codegen/sdk/typescript/file/test_file_remove.py +++ b/tests/unit/codegen/sdk/typescript/file/test_file_remove.py @@ -97,7 +97,7 @@ def test_remove_unused_imports_with_moved_symbols(tmpdir): # Move foo to a new file new_file = codebase.create_file("new.ts") - foo.move_to_file(new_file,cleanup_unused_imports=False) + foo.move_to_file(new_file, cleanup_unused_imports=False) codebase.commit() # Confirm cleanup false is respected assert main_file.content.strip() == "import { helper } from './utils';" diff --git a/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py b/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py index 6ad356dad..9042477cf 100644 --- a/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py +++ b/tests/unit/codegen/sdk/typescript/file/test_file_unicode.py @@ -47,7 +47,7 @@ def test_unicode_move_symbol(tmpdir) -> None: file3 = codebase.get_file("file3.ts") bar = file2.get_function("bar") - bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge",cleanup_unused_imports=False) + bar.move_to_file(file3, include_dependencies=True, strategy="add_back_edge", cleanup_unused_imports=False) assert file1.content == content1 # language=typescript From fbfdb53bb7519a1d589ce435fa3588e80ba96ba5 Mon Sep 17 00:00:00 2001 From: tomcodegen Date: Tue, 18 Mar 2025 09:36:38 -0700 Subject: [PATCH 3/6] trim --- src/codegen/sdk/core/symbol.py | 243 ++++++++++++--------------- src/codegen/sdk/typescript/symbol.py | 193 +++++++++++++-------- 2 files changed, 236 insertions(+), 200 deletions(-) diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index 2a10555bb..ea39b8071 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -267,6 +267,38 @@ def insert_before(self, new_src: str, fix_indentation: bool = False, newline: bo return first_node.insert_before(new_src, fix_indentation, newline, priority, dedupe) return super().insert_before(new_src, fix_indentation, newline, priority, dedupe) + def _post_move_import_cleanup(self,encountered_symbols,strategy): + # =====[ Remove any imports that are no longer used ]===== + from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: + if strategy != "duplicate_dependencies": + other_usages = [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] + else: + other_usages = [usage.usage_symbol for usage in dep.usages] + if isinstance(dep, Import): + dep.remove_if_unused() + + if not other_usages: + # Clean up forward... + dep.remove() + elif isinstance(dep, Symbol): + usages_in_file = [ + symb + for symb in other_usages + if symb.file == self.file and not self.transaction_manager.get_transaction_containing_range(symb.file.path, symb.start_byte, symb.end_byte, TransactionPriority.Remove) + ] + if self.transaction_manager.get_transaction_containing_range(dep.file.path, dep.start_byte, dep.end_byte, transaction_order=TransactionPriority.Remove): + if not usages_in_file and strategy != "add_back_edge": + # We are going to assume there is only one such import + if imp_list := [import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: + if insert_import_list := [ + transaction + for transaction in self.transaction_manager.queued_transactions[self.file.path] + if imp_list[0] and transaction.new_content and imp_list[0] in transaction.new_content and transaction.transaction_order == TransactionPriority.Insert + ]: + self.transaction_manager.queued_transactions[self.file.path].remove(insert_import_list[0]) + self.file._pending_imports.remove(imp_list[0]) + def move_to_file( self, file: SourceFile, @@ -292,91 +324,7 @@ def move_to_file( AssertionError: If an invalid strategy is provided. """ encountered_symbols = {self} - self._move_to_file(file, encountered_symbols, include_dependencies, strategy, cleanup_unused_imports) - - @noapidoc - def _move_dependencies( - self, file: SourceFile, encountered_symbols: set[Symbol | Import], strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports" - ): - from codegen.sdk.core.import_resolution import Import - - for dep in self.dependencies: - if dep in encountered_symbols: - continue - - # =====[ Symbols - move over ]===== - if isinstance(dep, Symbol) and dep.is_top_level: - encountered_symbols.add(dep) - dep._move_to_file( - file=file, - encountered_symbols=encountered_symbols, - include_dependencies=True, - strategy=strategy, - ) - - # =====[ Imports - copy over ]===== - elif isinstance(dep, Import): - if dep.imported_symbol: - file.add_import(imp=dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type) - else: - file.add_import(imp=dep.source) - - @noapidoc - def _update_dependencies_on_move(self, file: SourceFile): - from codegen.sdk.core.import_resolution import Import - - for dep in self.dependencies: - # =====[ Symbols - add back edge ]===== - if isinstance(dep, Symbol) and dep.is_top_level: - file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=False) - elif isinstance(dep, Import): - if dep.imported_symbol: - file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) - else: - file.add_import(imp=dep.source) - - @noapidoc - def _execute_post_move_correction_strategy( - self, - file: SourceFile, - encountered_symbols: set[Symbol | Import], - strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], - ): - from codegen.sdk.core.import_resolution import Import - - import_line = self.get_import_string(module=file.import_module_name) - is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) - match strategy: - case "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - case "add_back_edge": - if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.file.add_import(imp=import_line) - - # Delete the original symbol - self.remove() - case "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: - # Add updated import - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) - usage.usage_symbol.file.add_import(imp=import_line) - - # Add the import to the original file - if is_used_in_file: - self.file.add_import(imp=import_line) - # Delete the original symbol - self.remove() + self._move_to_file(file, encountered_symbols, include_dependencies, strategy,cleanup_unused_imports) @noapidoc def _move_to_file( @@ -388,71 +336,98 @@ def _move_to_file( cleanup_unused_imports: bool = True, ) -> tuple[NodeId, NodeId]: """Helper recursive function for `move_to_file`""" + from codegen.sdk.core.import_resolution import Import + # =====[ Arg checking ]===== if file == self.file: return file.file_node_id, self.node_id - - # This removes any imports in the file we're moving the symbol to if imp := file.get_import(self.name): encountered_symbols.add(imp) imp.remove() - # This handles the new file - # =====[ Move over dependencies recursively ]===== if include_dependencies: - self._move_dependencies(file, encountered_symbols, strategy) + # =====[ Move over dependencies recursively ]===== + for dep in self.dependencies: + if dep in encountered_symbols: + continue + + # =====[ Symbols - move over ]===== + if isinstance(dep, Symbol) and dep.is_top_level: + encountered_symbols.add(dep) + dep._move_to_file( + file=file, + encountered_symbols=encountered_symbols, + include_dependencies=include_dependencies, + strategy=strategy, + ) + + # =====[ Imports - copy over ]===== + elif isinstance(dep, Import): + if dep.imported_symbol: + file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) + else: + file.add_import(imp=dep.source) else: - self._update_dependencies_on_move(file) + for dep in self.dependencies: + # =====[ Symbols - add back edge ]===== + if isinstance(dep, Symbol) and dep.is_top_level: + file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=False) + elif isinstance(dep, Import): + if dep.imported_symbol: + file.add_import(imp=dep.imported_symbol, alias=dep.alias.source) + else: + file.add_import(imp=dep.source) # =====[ Make a new symbol in the new file ]===== - should_export = False - if hasattr(self, "is_exported") and ( - self.is_exported - or [ - usage - for usage in self.usages - if usage.usage_symbol not in encountered_symbols - and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) - ] - ): - should_export = True - file.add_symbol(self, should_export=should_export) + file.add_symbol(self) + import_line = self.get_import_string(module=file.import_module_name) + # =====[ Checks if symbol is used in original file ]===== # Takes into account that it's dependencies will be moved - from codegen.sdk.core.import_resolution import Import + is_used_in_file = any( + usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols and (usage.start_byte < self.start_byte or usage.end_byte > self.end_byte) # HACK + for usage in self.symbol_usages + ) + + # ======[ Strategy: Duplicate Dependencies ]===== + if strategy == "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.remove() - self._execute_post_move_correction_strategy(file, encountered_symbols, strategy) + # ======[ Strategy: Add Back Edge ]===== + # Here, we will add a "back edge" to the old file importing the symbol + elif strategy == "add_back_edge": + if is_used_in_file or any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): + self.file.add_import(imp=import_line) + # Delete the original symbol + self.remove() + + # ======[ Strategy: Update All Imports ]===== + # Update the imports in all the files which use this symbol to get it from the new file now + elif strategy == "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, Import) and usage.usage_symbol.file != file: + # Add updated import + usage.usage_symbol.file.add_import(import_line) + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(imp=import_line) + + # Add the import to the original file + if is_used_in_file: + self.file.add_import(imp=import_line) + # Delete the original symbol + self.remove() - # =====[ Remove any imports that are no longer used ]===== if cleanup_unused_imports: - for dep in self.dependencies: - if strategy != "duplicate_dependencies": - other_usages = [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] - else: - other_usages = [usage.usage_symbol for usage in dep.usages] - if isinstance(dep, Import): - dep.remove_if_unused() - - if not other_usages: - # Clean up forward... - dep.remove() - elif isinstance(dep, Symbol): - usages_in_file = [ - symb - for symb in other_usages - if symb.file == self.file and not self.transaction_manager.get_transaction_containing_range(symb.file.path, symb.start_byte, symb.end_byte, TransactionPriority.Remove) - ] - if self.transaction_manager.get_transaction_containing_range(dep.file.path, dep.start_byte, dep.end_byte, transaction_order=TransactionPriority.Remove): - if not usages_in_file and strategy != "add_back_edge": - # We are going to assume there is only one such import - if imp_list := [import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: - if insert_import_list := [ - transaction - for transaction in self.transaction_manager.queued_transactions[self.file.path] - if imp_list[0] and transaction.new_content and imp_list[0] in transaction.new_content and transaction.transaction_order == TransactionPriority.Insert - ]: - self.transaction_manager.queued_transactions[self.file.path].remove(insert_import_list[0]) - self.file._pending_imports.remove(imp_list[0]) + self._post_move_import_cleanup(encountered_symbols,strategy) @property @reader diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index e323222c3..1910ea039 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -1,7 +1,8 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Literal, Self, Unpack, override +from typing import TYPE_CHECKING, Literal, Self, Unpack +from codegen.sdk.codebase.transactions import TransactionPriority from codegen.sdk.core.assignment import Assignment from codegen.sdk.core.autocommit import reader, writer from codegen.sdk.core.dataclasses.usage import UsageKind, UsageType @@ -27,6 +28,7 @@ from codegen.sdk.core.file import SourceFile from codegen.sdk.core.import_resolution import Import from codegen.sdk.core.interfaces.editable import Editable + from codegen.sdk.core.node_id_factory import NodeId from codegen.sdk.typescript.detached_symbols.code_block import TSCodeBlock from codegen.sdk.typescript.interfaces.has_block import TSHasBlock @@ -254,87 +256,146 @@ def has_semicolon(self) -> bool: return self.semicolon_node is not None @noapidoc - @override - def _update_dependencies_on_move(self, file: SourceFile): - for dep in self.dependencies: - if isinstance(dep, Assignment): - msg = "Assignment not implemented yet" - raise NotImplementedError(msg) - - # =====[ Symbols - move over ]===== - elif isinstance(dep, TSSymbol) and dep.is_top_level: - file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=isinstance(dep, TypeAlias)) - - if not dep.is_exported: - dep.file.add_export_to_symbol(dep) - pass - # =====[ Imports - copy over ]===== - elif isinstance(dep, TSImport): - if dep.imported_symbol: - file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type, is_type_import=dep.is_type_import()) - else: - file.add_import(dep.source) - - @noapidoc - @override - def _execute_post_move_correction_strategy( + def _move_to_file( self, file: SourceFile, encountered_symbols: set[Symbol | Import], - strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"], - ): + include_dependencies: bool = True, + strategy: Literal["add_back_edge", "update_all_imports", "duplicate_dependencies"] = "update_all_imports", + cleanup_unused_imports: bool = True, + ) -> tuple[NodeId, NodeId]: + # TODO: Prevent creation of import loops (!) - raise a ValueError and make the agent fix it + # =====[ Arg checking ]===== + if file == self.file: + return file.file_node_id, self.node_id + + if imp := file.get_import(self.name): + encountered_symbols.add(imp) + imp.remove() + + # =====[ Move over dependencies recursively ]===== + if include_dependencies: + try: + for dep in self.dependencies: + if dep in encountered_symbols: + continue + + # =====[ Symbols - move over ]===== + elif isinstance(dep, TSSymbol): + if dep.is_top_level: + encountered_symbols.add(dep) + dep._move_to_file(file, encountered_symbols=encountered_symbols, include_dependencies=True, strategy=strategy) + + # =====[ Imports - copy over ]===== + elif isinstance(dep, TSImport): + if dep.imported_symbol: + file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type) + else: + file.add_import(dep.source) + + else: + msg = f"Unknown dependency type {type(dep)}" + raise ValueError(msg) + except Exception as e: + print(f"Failed to move dependencies of {self.name}: {e}") + else: + try: + for dep in self.dependencies: + if isinstance(dep, Assignment): + msg = "Assignment not implemented yet" + raise NotImplementedError(msg) + + # =====[ Symbols - move over ]===== + elif isinstance(dep, Symbol) and dep.is_top_level: + file.add_import(imp=dep, alias=dep.name, import_type=ImportType.NAMED_EXPORT, is_type_import=isinstance(dep, TypeAlias)) + + if not dep.is_exported: + dep.file.add_export_to_symbol(dep) + pass + + # =====[ Imports - copy over ]===== + elif isinstance(dep, TSImport): + if dep.imported_symbol: + file.add_import(dep.imported_symbol, alias=dep.alias.source, import_type=dep.import_type, is_type_import=dep.is_type_import()) + else: + file.add_import(dep.source) + + except Exception as e: + print(f"Failed to move dependencies of {self.name}: {e}") + + # =====[ Make a new symbol in the new file ]===== + # This will update all edges etc. + should_export = False + + + if self.is_exported or [ + usage + for usage in self.usages + if usage.usage_symbol not in encountered_symbols + and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) + ]: + should_export = True + + file.add_symbol(self, should_export=should_export) import_line = self.get_import_string(module=file.import_module_name) + # =====[ Checks if symbol is used in original file ]===== # Takes into account that it's dependencies will be moved is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL and usage not in encountered_symbols for usage in self.symbol_usages) - match strategy: - case "duplicate_dependencies": - # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol - is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL for usage in self.symbol_usages) - if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): - self.remove() - case "add_back_edge": - if is_used_in_file: - back_edge_line = import_line - if self.is_exported: - back_edge_line = back_edge_line.replace("import", "export") - self.file.add_import(back_edge_line) - elif self.is_exported: - module_name = file.name - self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") - # Delete the original symbol - self.remove() - case "update_all_imports": - for usage in self.usages: - if isinstance(usage.usage_symbol, TSImport): - # Add updated import - if usage.usage_symbol.resolved_symbol is not None and usage.usage_symbol.resolved_symbol.node_type == NodeType.SYMBOL and usage.usage_symbol.resolved_symbol == self: - if usage.usage_symbol.file != file: - # Just remove if the dep is now going to the file - usage.usage_symbol.file.add_import(import_line) - usage.usage_symbol.remove() - elif usage.usage_type == UsageType.CHAINED: - # Update all previous usages of import * to the new import name - if usage.match and "." + self.name in usage.match: - if isinstance(usage.match, FunctionCall): - usage.match.get_name().edit(self.name) - if isinstance(usage.match, ChainedAttribute): - usage.match.edit(self.name) - usage.usage_symbol.file.add_import(import_line) - - if is_used_in_file: - self.file.add_import(import_line) - # Delete the original symbol + # ======[ Strategy: Duplicate Dependencies ]===== + if strategy == "duplicate_dependencies": + # If not used in the original file. or if not imported from elsewhere, we can just remove the original symbol + is_used_in_file = any(usage.file == self.file and usage.node_type == NodeType.SYMBOL for usage in self.symbol_usages) + if not is_used_in_file and not any(usage.kind is UsageKind.IMPORTED and usage.usage_symbol not in encountered_symbols for usage in self.usages): self.remove() + # ======[ Strategy: Add Back Edge ]===== + # Here, we will add a "back edge" to the old file importing the self + elif strategy == "add_back_edge": + if is_used_in_file: + back_edge_line = import_line + if self.is_exported: + back_edge_line = back_edge_line.replace("import", "export") + self.file.add_import(back_edge_line) + elif self.is_exported: + module_name = file.name + self.file.add_import(f"export {{ {self.name} }} from '{module_name}'") + # Delete the original symbol + self.remove() + + # ======[ Strategy: Update All Imports ]===== + # Update the imports in all the files which use this symbol to get it from the new file now + elif strategy == "update_all_imports": + for usage in self.usages: + if isinstance(usage.usage_symbol, TSImport) and usage.usage_symbol.file != file: + # Add updated import + usage.usage_symbol.file.add_import(import_line) + usage.usage_symbol.remove() + elif usage.usage_type == UsageType.CHAINED: + # Update all previous usages of import * to the new import name + if usage.match and "." + self.name in usage.match: + if isinstance(usage.match, FunctionCall) and self.name in usage.match.get_name(): + usage.match.get_name().edit(self.name) + if isinstance(usage.match, ChainedAttribute): + usage.match.edit(self.name) + usage.usage_symbol.file.add_import(imp=import_line) + + # Add the import to the original file + if is_used_in_file: + self.file.add_import(imp=import_line) + # Delete the original symbol + self.remove() + if cleanup_unused_imports: + self._post_move_import_cleanup(encountered_symbols,strategy) + + def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter | None, level: int) -> str: """Converts a PropType definition to its TypeScript equivalent.""" # Handle basic types type_map = {"string": "string", "number": "number", "bool": "boolean", "object": "object", "array": "any[]", "func": "CallableFunction"} if prop_type.source in type_map: return type_map[prop_type.source] - if isinstance(prop_type, ChainedAttribute): if prop_type.attribute.source == "node": return "T" From 20c25064649dc3520c6ec7be3d0d89e7c8108b23 Mon Sep 17 00:00:00 2001 From: tomcodgen <191515280+tomcodgen@users.noreply.github.com> Date: Tue, 18 Mar 2025 16:37:35 +0000 Subject: [PATCH 4/6] Automated pre-commit update --- src/codegen/sdk/core/symbol.py | 7 ++++--- src/codegen/sdk/typescript/symbol.py | 14 ++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index ea39b8071..174ff37ab 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -267,9 +267,10 @@ def insert_before(self, new_src: str, fix_indentation: bool = False, newline: bo return first_node.insert_before(new_src, fix_indentation, newline, priority, dedupe) return super().insert_before(new_src, fix_indentation, newline, priority, dedupe) - def _post_move_import_cleanup(self,encountered_symbols,strategy): + def _post_move_import_cleanup(self, encountered_symbols, strategy): # =====[ Remove any imports that are no longer used ]===== from codegen.sdk.core.import_resolution import Import + for dep in self.dependencies: if strategy != "duplicate_dependencies": other_usages = [usage.usage_symbol for usage in dep.usages if usage.usage_symbol not in encountered_symbols] @@ -324,7 +325,7 @@ def move_to_file( AssertionError: If an invalid strategy is provided. """ encountered_symbols = {self} - self._move_to_file(file, encountered_symbols, include_dependencies, strategy,cleanup_unused_imports) + self._move_to_file(file, encountered_symbols, include_dependencies, strategy, cleanup_unused_imports) @noapidoc def _move_to_file( @@ -427,7 +428,7 @@ def _move_to_file( self.remove() if cleanup_unused_imports: - self._post_move_import_cleanup(encountered_symbols,strategy) + self._post_move_import_cleanup(encountered_symbols, strategy) @property @reader diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index 1910ea039..13135b3a4 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -327,13 +327,12 @@ def _move_to_file( # This will update all edges etc. should_export = False - if self.is_exported or [ - usage - for usage in self.usages - if usage.usage_symbol not in encountered_symbols - and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) - ]: + usage + for usage in self.usages + if usage.usage_symbol not in encountered_symbols + and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) + ]: should_export = True file.add_symbol(self, should_export=should_export) @@ -387,8 +386,7 @@ def _move_to_file( # Delete the original symbol self.remove() if cleanup_unused_imports: - self._post_move_import_cleanup(encountered_symbols,strategy) - + self._post_move_import_cleanup(encountered_symbols, strategy) def _convert_proptype_to_typescript(self, prop_type: Editable, param: Parameter | None, level: int) -> str: """Converts a PropType definition to its TypeScript equivalent.""" From 9acc65337856a076a05825eb75a5ea7fdc0331b1 Mon Sep 17 00:00:00 2001 From: tkucar Date: Thu, 20 Mar 2025 21:30:40 +0100 Subject: [PATCH 5/6] patch --- src/codegen/sdk/core/import_resolution.py | 18 ++- src/codegen/sdk/core/interfaces/editable.py | 11 +- src/codegen/sdk/core/symbol.py | 7 +- src/codegen/sdk/typescript/symbol.py | 3 +- .../function/test_function_move_to_file.py | 122 ++++++++++++++++-- uv.lock | 15 +-- 6 files changed, 142 insertions(+), 34 deletions(-) diff --git a/src/codegen/sdk/core/import_resolution.py b/src/codegen/sdk/core/import_resolution.py index 9f5fa8efa..8f684d866 100644 --- a/src/codegen/sdk/core/import_resolution.py +++ b/src/codegen/sdk/core/import_resolution.py @@ -5,7 +5,6 @@ from typing import TYPE_CHECKING, ClassVar, Generic, Literal, Self, TypeVar, override from codegen.sdk.codebase.resolution_stack import ResolutionStack -from codegen.sdk.codebase.transactions import TransactionPriority from codegen.sdk.core.autocommit import commiter, reader, remover, writer from codegen.sdk.core.dataclasses.usage import UsageKind from codegen.sdk.core.expressions.name import Name @@ -682,13 +681,24 @@ def __eq__(self, other: object): @noapidoc @reader - def remove_if_unused(self) -> None: + def remove_if_unused(self, force:bool=False) -> bool: + """Removes import if it is not being used. Considers current transaction removals. + + Args: + force (bool, optional): If true removes the import even if we cannot ascertain the usage for sure. Defaults to False. + + Returns: + bool: True if removed, False if not + """ if all( - self.transaction_manager.get_transaction_containing_range(self.file.path, start_byte=usage.match.start_byte, end_byte=usage.match.end_byte, transaction_order=TransactionPriority.Remove) + usage.match.get_transaction_if_pending_removal() for usage in self.usages ): + if not force and not self.usage_is_ascertainable(): + return False self.remove() - + return True + return False @noapidoc @reader def resolve_attribute(self, attribute: str) -> TSourceFile | None: diff --git a/src/codegen/sdk/core/interfaces/editable.py b/src/codegen/sdk/core/interfaces/editable.py index 86e08c844..ba13129bc 100644 --- a/src/codegen/sdk/core/interfaces/editable.py +++ b/src/codegen/sdk/core/interfaces/editable.py @@ -10,7 +10,7 @@ from rich.pretty import Pretty from codegen.sdk.codebase.span import Span -from codegen.sdk.codebase.transactions import EditTransaction, InsertTransaction, RemoveTransaction, TransactionPriority +from codegen.sdk.codebase.transactions import EditTransaction, InsertTransaction, RemoveTransaction, Transaction, TransactionPriority from codegen.sdk.core.autocommit import commiter, reader, remover, repr_func, writer from codegen.sdk.core.placeholder.placeholder import Placeholder from codegen.sdk.extensions.utils import get_all_identifiers @@ -1156,6 +1156,15 @@ def parent_class(self) -> Class | None: return self.parent_of_type(Class) + @noapidoc + def get_transaction_if_pending_removal(self) -> Transaction|None: + """Checks if this editable is being removed by some transaction and if so returns it. + + Returns: + Transaction|None: The transaction removing the editable + """ + return self.transaction_manager.get_transaction_containing_range(self.file.path, self.start_byte, self.end_byte, TransactionPriority.Remove) + def _get_ast_children(self) -> list[tuple[str | None, AST]]: children = [] names = {} diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index 174ff37ab..3e7311214 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -279,16 +279,13 @@ def _post_move_import_cleanup(self, encountered_symbols, strategy): if isinstance(dep, Import): dep.remove_if_unused() - if not other_usages: - # Clean up forward... - dep.remove() elif isinstance(dep, Symbol): usages_in_file = [ symb for symb in other_usages - if symb.file == self.file and not self.transaction_manager.get_transaction_containing_range(symb.file.path, symb.start_byte, symb.end_byte, TransactionPriority.Remove) + if symb.file == self.file and not symb.get_transaction_if_pending_removal() ] - if self.transaction_manager.get_transaction_containing_range(dep.file.path, dep.start_byte, dep.end_byte, transaction_order=TransactionPriority.Remove): + if dep.get_transaction_if_pending_removal(): if not usages_in_file and strategy != "add_back_edge": # We are going to assume there is only one such import if imp_list := [import_str for import_str in self.file._pending_imports if dep.name and dep.name in import_str]: diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index 13135b3a4..4470689c8 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -2,7 +2,6 @@ from typing import TYPE_CHECKING, Literal, Self, Unpack -from codegen.sdk.codebase.transactions import TransactionPriority from codegen.sdk.core.assignment import Assignment from codegen.sdk.core.autocommit import reader, writer from codegen.sdk.core.dataclasses.usage import UsageKind, UsageType @@ -331,7 +330,7 @@ def _move_to_file( usage for usage in self.usages if usage.usage_symbol not in encountered_symbols - and not self.transaction_manager.get_transaction_containing_range(usage.usage_symbol.file.path, usage.usage_symbol.start_byte, usage.usage_symbol.end_byte, TransactionPriority.Remove) + and not usage.usage_symbol.get_transaction_if_pending_removal() ]: should_export = True diff --git a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py index cefb0c802..34337bb8f 100644 --- a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py +++ b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py @@ -88,6 +88,100 @@ def bar(): assert file3.content.strip() == EXPECTED_FILE_3_CONTENT.strip() +def test_move_to_file_update_all_imports_multi_layer_usage(tmpdir) -> None: + # ========== [ BEFORE ] ========== + # language=python + FILE_1_CONTENT = """ +def external_dep(): + return 42 +""" + + # language=python + FILE_2_CONTENT = """ +from file1 import external_dep + +def foo(): + return foo_dep_wrapped() + foo_dep() + +def foo_dep_wrapped(): + return foo_dep()+2 + +def foo_dep(): + return 24 + +def bar(): + return external_dep() + bar_dep() + +def bar_dep(): + return 2 +""" + + # language=python + FILE_3_CONTENT = """ +from file2 import bar + +def baz(): + return bar() + 1 +""" + + # ========== [ AFTER ] ========== + # language=python + EXPECTED_FILE_1_CONTENT = """ +def external_dep(): + return 42 +""" + + # language=python + EXPECTED_FILE_2_CONTENT = """ +from file1 import external_dep + +def bar(): + return external_dep() + bar_dep() + +def bar_dep(): + return 2 +""" + + # language=python + EXPECTED_FILE_3_CONTENT = """ +from file2 import bar + +def baz(): + return bar() + 1 + +def foo_dep(): + return 24 + +def foo_dep_wrapped(): + return foo_dep()+2 + +def foo(): + return foo_dep_wrapped() + foo_dep() + +""" + # =============================== + # TODO: [low] Missing newline after import + + with get_codebase_session( + tmpdir=tmpdir, + files={ + "file1.py": FILE_1_CONTENT, + "file2.py": FILE_2_CONTENT, + "file3.py": FILE_3_CONTENT, + }, + ) as codebase: + file1 = codebase.get_file("file1.py") + file2 = codebase.get_file("file2.py") + file3 = codebase.get_file("file3.py") + + foo = file2.get_function("foo") + foo.move_to_file(file3, include_dependencies=True, strategy="update_all_imports") + + assert file1.content.strip() == EXPECTED_FILE_1_CONTENT.strip() + assert file2.content.strip() == EXPECTED_FILE_2_CONTENT.strip() + assert file3.content.strip() == EXPECTED_FILE_3_CONTENT.strip() + + def test_move_to_file_update_all_imports_include_dependencies(tmpdir) -> None: # ========== [ BEFORE ] ========== # language=python @@ -1483,19 +1577,26 @@ def test_move_to_file_decorators(tmpdir) -> None: # ========== [ BEFORE ] ========== # language=python FILE_1_CONTENT = """ - from test.foo import TEST +from test.foo import TEST - test_decorator = TEST() +test_decorator = TEST() - @test_decorator.foo() - def test_func(): - pass -""" +@test_decorator.foo() +def test_func(): + pass + """ - # language=python - FILE_2_CONTENT = """ + FILE_2_CONTENT = "" + EXPECTED_FILE_1_CONTENT = "" - """ + EXPECTED_FILE_2_CONTENT = """from test.foo import TEST + + +test_decorator = TEST() + +@test_decorator.foo() +def test_func(): + pass""" with get_codebase_session( tmpdir=tmpdir, @@ -1514,6 +1615,9 @@ def test_func(): file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") + assert file1.source==EXPECTED_FILE_1_CONTENT + assert file2.source==EXPECTED_FILE_2_CONTENT + def test_move_to_file_multiple_same_transaction(tmpdir) -> None: # language=python diff --git a/uv.lock b/uv.lock index 1991652aa..52e1020e8 100644 --- a/uv.lock +++ b/uv.lock @@ -1,4 +1,5 @@ version = 1 +revision = 1 requires-python = ">=3.12, <3.14" resolution-markers = [ "python_full_version >= '3.12.4'", @@ -549,7 +550,6 @@ dependencies = [ { name = "hatchling" }, { name = "httpx" }, { name = "humanize" }, - { name = "jwt" }, { name = "langchain", extra = ["openai"] }, { name = "langchain-anthropic" }, { name = "langchain-core" }, @@ -681,7 +681,6 @@ requires-dist = [ { name = "hatchling", specifier = ">=1.25.0" }, { name = "httpx", specifier = ">=0.28.1" }, { name = "humanize", specifier = ">=4.10.0,<5.0.0" }, - { name = "jwt", specifier = ">=1.3.1" }, { name = "langchain", extras = ["openai"] }, { name = "langchain-anthropic", specifier = ">=0.3.7" }, { name = "langchain-core" }, @@ -745,6 +744,7 @@ requires-dist = [ { name = "wrapt", specifier = ">=1.16.0,<2.0.0" }, { name = "xmltodict", specifier = ">=0.13.0,<1.0.0" }, ] +provides-extras = ["lsp", "types"] [package.metadata.requires-dev] dev = [ @@ -2019,17 +2019,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/54/09/2032e7d15c544a0e3cd831c51d77a8ca57f7555b2e1b2922142eddb02a84/jupyterlab_server-2.27.3-py3-none-any.whl", hash = "sha256:e697488f66c3db49df675158a77b3b017520d772c6e1548c7d9bcc5df7944ee4", size = 59700 }, ] -[[package]] -name = "jwt" -version = "1.3.1" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "cryptography" }, -] -wheels = [ - { url = "https://files.pythonhosted.org/packages/ad/66/1e792aef36645b96271b4d27c2a8cc9fc7bbbaf06277a849b9e1a6360e6a/jwt-1.3.1-py3-none-any.whl", hash = "sha256:61c9170f92e736b530655e75374681d4fcca9cfa8763ab42be57353b2b203494", size = 18192 }, -] - [[package]] name = "langchain" version = "0.3.20" From 88cc50d1002e529e6d023859410abbf8ddef8451 Mon Sep 17 00:00:00 2001 From: tomcodgen <191515280+tomcodgen@users.noreply.github.com> Date: Thu, 20 Mar 2025 20:31:33 +0000 Subject: [PATCH 6/6] Automated pre-commit update --- src/codegen/sdk/core/import_resolution.py | 8 +++----- src/codegen/sdk/core/interfaces/editable.py | 2 +- src/codegen/sdk/core/symbol.py | 6 +----- src/codegen/sdk/typescript/symbol.py | 7 +------ .../sdk/python/function/test_function_move_to_file.py | 4 ++-- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/codegen/sdk/core/import_resolution.py b/src/codegen/sdk/core/import_resolution.py index 8f684d866..1fb17df50 100644 --- a/src/codegen/sdk/core/import_resolution.py +++ b/src/codegen/sdk/core/import_resolution.py @@ -681,7 +681,7 @@ def __eq__(self, other: object): @noapidoc @reader - def remove_if_unused(self, force:bool=False) -> bool: + def remove_if_unused(self, force: bool = False) -> bool: """Removes import if it is not being used. Considers current transaction removals. Args: @@ -690,15 +690,13 @@ def remove_if_unused(self, force:bool=False) -> bool: Returns: bool: True if removed, False if not """ - if all( - usage.match.get_transaction_if_pending_removal() - for usage in self.usages - ): + if all(usage.match.get_transaction_if_pending_removal() for usage in self.usages): if not force and not self.usage_is_ascertainable(): return False self.remove() return True return False + @noapidoc @reader def resolve_attribute(self, attribute: str) -> TSourceFile | None: diff --git a/src/codegen/sdk/core/interfaces/editable.py b/src/codegen/sdk/core/interfaces/editable.py index ba13129bc..0a8c6dd67 100644 --- a/src/codegen/sdk/core/interfaces/editable.py +++ b/src/codegen/sdk/core/interfaces/editable.py @@ -1157,7 +1157,7 @@ def parent_class(self) -> Class | None: return self.parent_of_type(Class) @noapidoc - def get_transaction_if_pending_removal(self) -> Transaction|None: + def get_transaction_if_pending_removal(self) -> Transaction | None: """Checks if this editable is being removed by some transaction and if so returns it. Returns: diff --git a/src/codegen/sdk/core/symbol.py b/src/codegen/sdk/core/symbol.py index 3e7311214..bce4a91e1 100644 --- a/src/codegen/sdk/core/symbol.py +++ b/src/codegen/sdk/core/symbol.py @@ -280,11 +280,7 @@ def _post_move_import_cleanup(self, encountered_symbols, strategy): dep.remove_if_unused() elif isinstance(dep, Symbol): - usages_in_file = [ - symb - for symb in other_usages - if symb.file == self.file and not symb.get_transaction_if_pending_removal() - ] + usages_in_file = [symb for symb in other_usages if symb.file == self.file and not symb.get_transaction_if_pending_removal()] if dep.get_transaction_if_pending_removal(): if not usages_in_file and strategy != "add_back_edge": # We are going to assume there is only one such import diff --git a/src/codegen/sdk/typescript/symbol.py b/src/codegen/sdk/typescript/symbol.py index 4470689c8..903fd8806 100644 --- a/src/codegen/sdk/typescript/symbol.py +++ b/src/codegen/sdk/typescript/symbol.py @@ -326,12 +326,7 @@ def _move_to_file( # This will update all edges etc. should_export = False - if self.is_exported or [ - usage - for usage in self.usages - if usage.usage_symbol not in encountered_symbols - and not usage.usage_symbol.get_transaction_if_pending_removal() - ]: + if self.is_exported or [usage for usage in self.usages if usage.usage_symbol not in encountered_symbols and not usage.usage_symbol.get_transaction_if_pending_removal()]: should_export = True file.add_symbol(self, should_export=should_export) diff --git a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py index 34337bb8f..a4c29dcdc 100644 --- a/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py +++ b/tests/unit/codegen/sdk/python/function/test_function_move_to_file.py @@ -1615,8 +1615,8 @@ def test_func(): file1 = codebase.get_file("file1.py") file2 = codebase.get_file("file2.py") - assert file1.source==EXPECTED_FILE_1_CONTENT - assert file2.source==EXPECTED_FILE_2_CONTENT + assert file1.source == EXPECTED_FILE_1_CONTENT + assert file2.source == EXPECTED_FILE_2_CONTENT def test_move_to_file_multiple_same_transaction(tmpdir) -> None: